what is wrong with the following very short program

W

wkevin

Hi All,

Any ideas what is wrong with the following very short program ?

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

typedef struct my_struct myStruct;
struct my_struct {
int val;
};



myStruct *f2(void)
{
myStruct *dst = (myStruct*)calloc(5, sizeof(myStruct));
dst->val = 5;
return dst;
}


void f1(myStruct *dst)
{
dst = f2();
printf("dst->val=%d\n", dst->val);
}


int main()
{
myStruct *src = (myStruct*)malloc(sizeof(myStruct));
myStruct *dst;
f1(dst);
printf("dst->val in main=%d\n", dst->val);

}


Any ideas why, when running this program, I get:

dst->val=5
dst->val in main=1

Why is it 1 in main and not 5 ?
I would expect that in main it will also be 5.
What should I do so that in main it will be 5 ?

regards,
Kevin
 
W

wkevin

Hi,
Thanks for the quick response.

Still, after fixing according to 4.8, the following does not work .Any ideas what should be done to fix it ?

typedef struct my_struct myStruct;
struct my_struct {
int val;
};



myStruct *f2(void)
{
static myStruct dst;
dst.val = 6;
return &dst;
}


void f1(myStruct *dst)
{
dst = f2();
printf("dst->val=%d\n", dst->val);
}


int main()
{
myStruct dst;
f1(&dst);
printf("**dst.val in main=%d\n", dst.val);

}
 
J

Jorgen Grahn

Hi All,

Any ideas what is wrong with the following very short program ?

[...]

When I compile it with the appropriate options set, gcc says:

foo.c: In function 'main':
foo.c:29:19: warning: unused variable 'src'
foo.c:31:11: warning: 'dst' is used uninitialized in this function

Let the compiler do its job! You have better things to do.

/Jorgen
 
E

Eric Sosman

Hi,
Thanks for the quick response.

Still, after fixing according to 4.8, [he's referring to FAQ 4.8]
the following does not work .Any ideas what should be done to fix it ?
[...]

The changes you made are not "according to 4.8" at all, which
leads me to suspect you didn't understand 4.8's point. Go back and
read it again, giving more attention to the answer's first sentence
than to the stuff that follows.
 
W

wkevin

Sorry, I read it. I am a newbie to C. I don't know what should be
done to fix the program. I will appreciate of anyone can fix this
very short program so that it will work

Regards,
Kevin
 
B

BartC

void f1(myStruct *dst)
{
dst = f2();
printf("dst->val=%d\n", dst->val);
}

Change f1() to:

myStruct *f1(void) {
myStruct *dst;
dst = f2();
printf("dst->val=%d\n", dst->val);
return dst;
}
int main()
{
myStruct *src = (myStruct*)malloc(sizeof(myStruct));
myStruct *dst;
f1(dst);

Change the above to:

dst=f1();

The problem is that dst in main() is not being set up to point to anything.
(You got the right idea with f2(), but interposing f1() between main() and
f2() wasn't done properly.)
dst->val in main=1

I just got a crash because dst is not set up.
 
J

James Kuyper

Hi All,

Any ideas what is wrong with the following very short program ?

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

typedef struct my_struct myStruct;
struct my_struct {
int val;
};



myStruct *f2(void)
{
myStruct *dst = (myStruct*)calloc(5, sizeof(myStruct));

calloc() can fail. If it does, it returns a null pointer. If dst is
null, then the following statement has undefined behavior. Always check
for the possibility of failure whenever calling malloc(), calloc(), or
realloc(), and make sure NOT to dereference the pointers they return if
they are null.
dst->val = 5;
return dst;
}


void f1(myStruct *dst)
{
dst = f2();
printf("dst->val=%d\n", dst->val);

The variable dst in this program contains a copy of the pointer value
passed to it by the calling routine. It then replaces that pointer with
the one returned by f2(). As a result, there's no point in having the
caller pass that value. You might as well define dst inside of f1,
rather than making it a function argument. That's not the correct way to
fix this problem, but it's a key symptom of what's wrong with this function.

When f1 returns, the variable named "dst" declared in this function
ceases to exist, and the information stored in that value is lost.
Therefore, it's no longer possible to free() the memory allocated by
f2(). This is a memory leak. That's less important than the other
problem, but s
}


int main()
{
myStruct *src = (myStruct*)malloc(sizeof(myStruct));
myStruct *dst;

At this point, dst is uninitialized; that means that it's value is
indeterminate.

Since the value of dst is indeterminate, it might be a trap
representation, a value that does not represent any valid memory
location. Attempting to read a trap representation has undefined
behavior. There are real world machines where that could cause your
program to be aborted.

That statement causes a the value of dst in main() to be copied to the
variable named dst in f1(). There is otherwise no connection between
those two variables, they are two entirely different variables that
happen to have the same name.

Therefore, the fact that f1() changes the value of f1()'s variable named
"dst" has no affect whatsoever on the value of main()'s variable named
dst. That variable remains is uninitialized.
printf("dst->val in main=%d\n", dst->val);

Even if dst doesn't contain a trap representation, it points at some
unknown location on your machine. That location is almost certainly NOT
the same as the location allocated by the call to calloc() in f2(). It
might not be a piece of memory that your program has no right to access;
attempting to access it could get your program aborted. The behavior of
attempting to dereference that pointer in order to retrieve the value of
"val" is undefined.
It's also possible that whatever memory is stored at that location,
contains a trap representation for "int". That's less likely than the
pointer trap representation, but still possible. If so, that provides a
third reason why the behavior of your program is undefined.
}


Any ideas why, when running this program, I get:

dst->val=5
dst->val in main=1

Why is it 1 in main and not 5 ?
I would expect that in main it will also be 5.
What should I do so that in main it will be 5 ?

f1() needs to have some way of transmitting the pointer that it gets
from f2() back to main(). Here's the two best ways to do this:

1: return the pointer as the value of f1():

myStruct *f1(void)
{
myStruct *dst = f2();
printf("dst->val=%d\n", dst->val);
return dst;
}

in main():

dst = f1();

// after you're done with dst:
free(dst);


2. Pass a pointer to dst to f1(), and use that pointer to fill in dst:

void f1(myStruct **pdst)
{
if(pdst == NULL)
{
// Insert error handling here
return;
}
*pdst = f2();
}

in main():

f1(&dst);

// after you're done with dst:
free(dst);
 
E

Eric Sosman

Sorry, I read it. I am a newbie to C. I don't know what should be
done to fix the program. I will appreciate of anyone can fix this
very short program so that it will work

(How can anyone "fix" it when we don't know what it's
supposed to do? ...)

Your problem: You haven't grasped what "pass by value"
means. A useful way to think about it is to consider the
function and its call separately:

- The function has some number of /parameters/ of assorted
types. Each parameter is very much like a local variable: It
comes into existence when the function is called, and it ceases
to exist when the function returns.

- The function call has some number of /arguments/. These
are expressions (sometimes rather trivial expressions) that are
evaluated to produce values. The results -- not the expressions
that produced them -- provide the initial values for the function's
parameters for the invocation caused by the call.

Got that? Argument expressions in the call produce values,
the function parameters come into existence and are initialized
with those values, the function executes and eventually returns,
and the parameters disappear.

So: If a function assigns a new value to a parameter, what
happens to the function's caller?

do do
sol sol sol so-o-o-l ...
do

Now, now, put your answer in the form of a question ... That's
correct: "What is Nothing At All?" is the right answer!

When your function f1() assigns a new value to its parameter
`dst', Nothing At All happens to the `dst' in main(), in either
version. (The first version of main() had another problem: It
used its own `dst' variable as an argument to initialize the `dst'
parameter in f1(), which meant that main()'s `dst' had to be
evaluated to get the value -- but main()'s `dst' had never been
given any value, so its value was "indeterminate" (often called
"garbage"), and even trying to evaluate it could get you in trouble.)

If you're going to write C (actually, if you're going to write
programs in any language I've ever encountered or even heard of),
you need to acquire some level of understanding of the tool you're
trying to use. Just fumbling around may eventually get you somewhere,
but the chances of it getting you somewhere you want to be are slim.
 
W

wkevin

Hi,
Thanks for your answer!
The problem is that I cannot
change f1(); it is a constraint which I have to adhere to!

Is there some workaround to achieve my goal ?

regards,
Kevin
 
J

James Kuyper

On 02/10/2014 04:56 PM, James Kuyper wrote:
....
... It
might not be a piece of memory that your program has no right to access;

While technically true ;-}, that's not what I meant to say. Either
remove the "not", or replace "no" with "a", whichever you prefer.
 
E

Eric Sosman

Hi,
Thanks for your answer!
The problem is that I cannot
change f1(); it is a constraint which I have to adhere to!

Is there some workaround to achieve my goal ?

Your goal being ...?
 
J

James Kuyper

Hi,
Thanks for your answer!
The problem is that I cannot
change f1(); it is a constraint which I have to adhere to!

I seriously doubt that. f1() contains several serious design flaws, as
indicated in my previous message. If you can't fix those design flaws,
there's only a limited number of pretty much useless things you can do
with f1(), none of which are likely to be what you want to do. It can't
have been written by a competent C programmer. If you can't modify it,
you'll have to get the person who does have a right to modify it to fix
those problems for you.
 
B

BartC

The problem is that I cannot
change f1(); it is a constraint which I have to adhere to!

Is there some workaround to achieve my goal ?

You have to start using pointers to pointers then: eg. myStruct **dst as the
parameter to f1(). Unless the constraint applies to that too?
 
E

Eric Sosman

You have to start using pointers to pointers then: eg. myStruct **dst as
the parameter to f1(). Unless the constraint applies to that too?

If he can't change f1(), he can't change the parameter.
(And if he changed the parameter as you suggest but could not
change the statements inside the function, he'd get a compile-
time error.)
 
R

Robbie Brown

Hi All,

Any ideas what is wrong with the following very short program ?

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

typedef struct my_struct myStruct;
struct my_struct {
int val;
};



myStruct *f2(void)
{
myStruct *dst = (myStruct*)calloc(5, sizeof(myStruct));
dst->val = 5;
return dst;
}


void f1(myStruct *dst)
{
dst = f2();
printf("dst->val=%d\n", dst->val);
}


int main()
{
myStruct *src = (myStruct*)malloc(sizeof(myStruct));
myStruct *dst;
f1(dst);
printf("dst->val in main=%d\n", dst->val);

}


Any ideas why, when running this program, I get:

dst->val=5
dst->val in main=1

Why is it 1 in main and not 5 ?
I would expect that in main it will also be 5.
What should I do so that in main it will be 5 ?

regards,
Kevin

int main(){
myStruct *dst = f2();
printf("dst->val in main=%d\n", dst->val);
}

no need to change f1()
 
J

James Kuyper

On 02/11/2014 12:25 PM, Robbie Brown wrote:
....
int main(){
myStruct *dst = f2();
printf("dst->val in main=%d\n", dst->val);
}

no need to change f1()

He wasn't entirely clear on the matter, but I've been assuming that the
constraint was not only that he couldn't change f1(), but that he was
also required to meaningfully use it. Otherwise, the constraint would be
meaningless - he would be free to create f3(), equivalent to f1(), but
modified. That is, essentially, what you've done, except that you've put
the code in main() instead of a separate function.
 
R

Robbie Brown

On 02/11/2014 12:25 PM, Robbie Brown wrote:
...

He wasn't entirely clear on the matter, but I've been assuming that the
constraint was not only that he couldn't change f1(),

Never assume anything ... or so my wizened old professor told me back in
the mists of time.
but that he was
also required to meaningfully use it. Otherwise, the constraint would be
meaningless - he would be free to create f3(), equivalent to f1(), but
modified. That is, essentially, what you've done, except that you've put
the code in main() instead of a separate function.

There are various ways of doing this aren't there. The two
constraints/requirements that I got were 'can't change f1()' and
'output 5 in main'. My solution requires the minimum of changes to
achieve the required result. But you are right of course, we don't have
the required background to be certain about anything. Still, given the
information we *have* got I'm quite happy with my solution. I'd argue
the point with the examiner but I defer to your (far) greater experience.
 
J

James Kuyper

On 02/11/2014 01:14 PM, Robbie Brown wrote:
....
There are various ways of doing this aren't there. The two
constraints/requirements that I got were 'can't change f1()' and
'output 5 in main'. My solution requires the minimum of changes to
achieve the required result. But you are right of course, we don't have
the required background to be certain about anything. Still, given the
information we *have* got I'm quite happy with my solution. I'd argue
the point with the examiner but I defer to your (far) greater experience.

My experience suggests that something got garbled during the
transmission of this constraint from whoever imposed it, through wkevin,
to us. I'm not sure what got garbled, but something has to have been.
 
K

Keith Thompson

James Kuyper said:
On 02/11/2014 01:14 PM, Robbie Brown wrote:
...

My experience suggests that something got garbled during the
transmission of this constraint from whoever imposed it, through wkevin,
to us. I'm not sure what got garbled, but something has to have been.

Agreed. And in such cases, it's usually a waste of time to proceed
without clarification from the person asking the question. (Often such
clarification is not forthcoming, which leads me to the conclusion that
it must not have been that important.)
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,039
Messages
2,570,376
Members
47,029
Latest member
EmiliaSton

Latest Threads

Top