malloc and functions

[raphfrk]

I am having an issue with malloc and gcc. Is there something wrong
with my code or is this a compiler bug ?

I am running this program:

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

typedef struct pxl {
double lon, lat;
double x,y,z;
double px,py;
} pixel;


struct chn {
int index;
struct nde *node;
};

struct nde {
pixel position;
struct chn *forward;
struct chn *reverse;
int *chain_num;
int size;
};

typedef struct chn chain;
typedef struct nde node;

node *createnode( );

main()
{

node *startn;

startn = createnode( );

startn->size = 2;
startn->forward = malloc( 2 * sizeof( chain * ) );
startn->reverse = malloc( 2 * sizeof( chain * ) );

printf("sf %p\nsr %p\n\n",
startn->forward,
startn->reverse
);

printf("sf %p\nsr %p\n\n",
startn->forward,
startn->reverse
);

}

node *createnode( )
{
node *node;
node = malloc( sizeof(node) );
return node;
}



I get the following output.

sf 0x660168
sr 0x660178

sf 0xa383731
sr 0x660178

The startn->forward point changes between the two printfs despite
there being no code between them.

[Richard]

Re: malloc and functions

raphfrk <raphfrk@netsca pe.netwrites:

>
node *createnode( )
{
node *node;
node = malloc( sizeof(node) );

Just a quick glance, the above "confused me".

node? Try "node * pnode" ...

No idea if it will fix your problem.

[raphfrk]

Re: malloc and functions

On Jul 15, 4:09 pm, Richard <rgr...@gmail.c omwrote:

raphfrk <raph...@netsca pe.netwrites:
>

node *createnode( )
{
node *node;
node = malloc( sizeof(node) );

>
Just a quick glance, the above "confused me".
>
node? Try "node * pnode" ...
>
No idea if it will fix your problem.

Yeah, that fixed it. I have been trying to figure it out for ages
(well about 3 hours). gcc must be misinterpreting what I wanted. It
didn't give a warning though.

Thanks alot.

[Robert Gamble]

Re: malloc and functions

On Jul 15, 10:57 am, raphfrk <raph...@netsca pe.netwrote:

I am having an issue with malloc and gcc. Is there something wrong
with my code or is this a compiler bug ?
>
I am running this program:
>
#include <stdio.h>
#include <stdlib.h>
>
typedef struct pxl {
double lon, lat;
double x,y,z;
double px,py;
} pixel;
>
struct chn {
int index;
struct nde *node;
};
>
struct nde {
pixel position;
struct chn *forward;
struct chn *reverse;
int *chain_num;
int size;
};
>
typedef struct chn chain;
typedef struct nde node;
>
node *createnode( );
>
main()

'int main (void)' is better.

{
>
node *startn;
>
startn = createnode( );
>
startn->size = 2;
startn->forward = malloc( 2 * sizeof( chain * ) );
startn->reverse = malloc( 2 * sizeof( chain * ) );
>
printf("sf %p\nsr %p\n\n",
startn->forward,
startn->reverse
);
>
printf("sf %p\nsr %p\n\n",
startn->forward,
startn->reverse
);

The %p conversion specifier expects a void * argument, you need to
cast your arguments appropriately.

}
>
node *createnode( )
{
node *node;

Yuck. Using the same name for a typedef and a variable (esp. one that
is a pointer to that type) is ugly and has the potential to cause
great confusion, see below.

node = malloc( sizeof(node) );

Hint: Does sizeof apply to the typedef name or the variable name?
It's probably not what you expect.
You should also check the return value of malloc for failure.

return node;
}
>
I get the following output.
>
sf 0x660168
sr 0x660178
>
sf 0xa383731
sr 0x660178
>
The startn->forward point changes between the two printfs despite
there being no code between them.

Fix the issues mentioned above and let us know if you still have a
problem.

Robert Gamble

[rzed]

Re: malloc and functions

raphfrk <raphfrk@netsca pe.netwrote in
news:1184512773 .598589.183920@ d55g2000hsg.goo glegroups.com:

On Jul 15, 4:09 pm, Richard <rgr...@gmail.c omwrote:

>raphfrk <raph...@netsca pe.netwrites:
>>

node *createnode( )
{
node *node;
node = malloc( sizeof(node) );

>>
>Just a quick glance, the above "confused me".
>>
>node? Try "node * pnode" ...
>>
>No idea if it will fix your problem.

>
Yeah, that fixed it. I have been trying to figure it out for
ages (well about 3 hours). gcc must be misinterpreting what I
wanted. It didn't give a warning though.
>
Thanks alot.
>

It may have fixed it, but it doesn't explain it. Your call to
createnode came before the first printf. So why does the confusion
matter to the printf?

By the way, I repeated the printf more times, and each time after
the first, it prints the same values as the second printf. That
is, it doesn't change after that first time. What could possibly
be happening under the hood to get that bizarre result?

--
rzed

[Richard]

Re: malloc and functions

raphfrk <raphfrk@netsca pe.netwrites:

On Jul 15, 4:09 pm, Richard <rgr...@gmail.c omwrote:

>raphfrk <raph...@netsca pe.netwrites:
>>

node *createnode( )
{
node *node;
node = malloc( sizeof(node) );

>>
>Just a quick glance, the above "confused me".
>>
>node? Try "node * pnode" ...
>>
>No idea if it will fix your problem.

>
Yeah, that fixed it. I have been trying to figure it out for ages
(well about 3 hours). gcc must be misinterpreting what I wanted. It
didn't give a warning though.

Rubbish in rubbish out :-; Personally, even if it ever was "ok" I would
never have a variable with the same name as a type. A variable should
reflect the object to which it refers. In this case a pointer to a node
- hence a "pnode".

>
Thanks alot.
>

No probs. Glad to help. Good luck.

Ask in a gcc newsgroup here about the warnings - seems strange:

gnu.gcc.help

[Kenny McCormack]

Re: malloc and functions

In article <87ejj9isnb.fsf @gmail.com>, Richard <rgrdev@gmail.c omwrote:
....

>Rubbish in rubbish out :-; Personally, even if it ever was "ok" I would
>never have a variable with the same name as a type.

This is common with structs. You frequently see code like: struct stat stat;
Of course: int int
doesn't work as well.

>A variable should reflect the object to which it refers. In this case a
>pointer to a node - hence a "pnode".

[Robert Gamble]

Re: malloc and functions

On Jul 15, 11:48 am, gaze...@xmissio n.xmission.com (Kenny McCormack)
wrote:

In article <87ejj9isnb.... @gmail.com>, Richard <rgr...@gmail.c omwrote:
>
...
>

Rubbish in rubbish out :-; Personally, even if it ever was "ok" I would
never have a variable with the same name as a type.

>
This is common with structs. You frequently see code like: struct stat stat;

This is true but the issue is much worse with typedefs since they
share the same namespace as ordinary identifiers and thus have greater
potential for confusion.

Of course: int int
doesn't work as well.

Well that's because int is a keyword.

typedef int INT;
{
INT INT;
....
}

This "works" but like the example presented by the OP is horrible
form.

Robert Gamble

[Richard]

Re: malloc and functions

gazelle@xmissio n.xmission.com (Kenny McCormack) writes:

In article <87ejj9isnb.fsf @gmail.com>, Richard <rgrdev@gmail.c omwrote:
...

>>Rubbish in rubbish out :-; Personally, even if it ever was "ok" I would
>>never have a variable with the same name as a type.

>
This is common with structs. You frequently see code like: struct stat stat;
Of course: int int
doesn't work as well.

I simply don't like it. More than likely it will confuse a debugger too
never mind someone maintaining your code.

[Thad Smith]

Re: malloc and functions

rzed wrote:

raphfrk <raphfrk@netsca pe.netwrote in
news:1184512773 .598589.183920@ d55g2000hsg.goo glegroups.com:
>

>>On Jul 15, 4:09 pm, Richard <rgr...@gmail.c omwrote:
>>

>>>raphfrk <raph...@netsca pe.netwrites:
>>>
>>>>node *createnode( )
>>>{
>>>node *node;
>>>node = malloc( sizeof(node) );
>>>
>>>Just a quick glance, the above "confused me".
>>>
>>>node? Try "node * pnode" ...
>>>
>>>No idea if it will fix your problem.

>>
>>Yeah, that fixed it. I have been trying to figure it out for
>>ages (well about 3 hours). gcc must be misinterpreting what I
>>wanted.

That's /one/ way of saying that you don't know the language.

It didn't give a warning though.

Try turning up the warnings. If you still doesn't warn, try using lint.
You are allowed to redeclare identifiers in an inner scope. It has good
uses, but pitfalls as well. You found one of the pitfalls.

It may have fixed it, but it doesn't explain it. Your call to
createnode came before the first printf. So why does the confusion
matter to the printf?

The error did not allocate sufficient space for struct nde.
Initializing that struct in the following code placed values into
unallocated space. printf probably used that space.

By the way, I repeated the printf more times, and each time after
the first, it prints the same values as the second printf. That
is, it doesn't change after that first time. What could possibly
be happening under the hood to get that bizarre result?

Writing beyond the allocated bounds of an object often has surprising
results.

--
Thad

[Martin Ambuhl]

Re: malloc and functions

raphfrk wrote:

I am having an issue with malloc and gcc. Is there something wrong
with my code or is this a compiler bug ?

[...]

node *createnode( )
{
node *node;
node = malloc( sizeof(node) );
return node;
}

Giving variables with the same identifier as one you use for a type is
just asking for trouble. Compare the above with

node *createnode( )
{
node *nodeptr;
nodeptr = malloc(sizeof(n ode));
return nodeptr;
}

or with

node *createnode( )
{
node *nodeptr;
nodeptr = malloc(sizeof *nodeptr);
return nodeptr;
}

[raphfrk]

Re: malloc and functions

On Jul 15, 4:34 pm, Robert Gamble <rgambl...@gmai l.comwrote:

On Jul 15, 10:57 am, raphfrk <raph...@netsca pe.netwrote:
>

node = malloc( sizeof(node) );

>
Hint: Does sizeof apply to the typedef name or the variable name?
It's probably not what you expect.
You should also check the return value of malloc for failure.

This is the main problem. I went through the original program and
checked every malloc assignment. I didn't get the sizeof argument
correct in a few other places.

I had one for an array of the form:

char *array;

array = malloc( 100*sizeof( char * ) )

Fix the issues mentioned above and let us know if you still have a
problem.
>
Robert Gamble

It seems to have fixed it (and other bug), by checking every sizeof
one at a time.

Thanks alot all.

[Chris Torek]

Re: malloc and functions

In article <1184511435.763 565.302190@o61g 2000hsh.googleg roups.com>
raphfrk <raphfrk@netsca pe.netwrote:

>I am having an issue with malloc and gcc.

Issues with malloc() quite often boil down to one or more of
three things:

- Failure to include <stdlib.h>. This one is short and mostly
self-explanatory.

- Not using the "comp.lang. c Standard Approved Method" of
calling malloc() :-) The "approved method" says that if
you a pointer variable "p" of type "T *", i.e.:

T *p;

the call to malloc() should have the form:

p = malloc(N * sizeof *p);

where N is the number of items to allocate -- if this is 1
it may be omitted -- and "sizeof *p" is literally just that:
the keyword "sizeof", the unary "*" operator, and the name
of the variable on the left of the "=" sign.

The sizeof operator (and its argument, *p in this case) can
be omitted only if "T" is some variant of "char". Since
sizeof(char), sizeof(signed char), and sizeof(unsigned char)
are all 1 byte definition, and N*1 is just N, it is OK to
write:

char *copy;
...
copy = malloc(strlen(o riginal) + 1);

even though the "c.l.c. Standard Approved Method" would have
one write:

copy = malloc((strlen( original) + 1) * sizeof *copy);

- Forgetting to add one to strlen(some_str ing) to account
for the fact that a string whose length is L requires L+1
bytes of storage. (For instance, the empty string, "", has
zero length and requires one byte to store its '\0'; a
string of length three like "abc" requires four bytes to
store 'a', 'b', 'c', and '\0'; and so on.)

In this case, the first and last seem to be OK here, but the
middle is a big problem.

>I am running this program:
>
>#include <stdio.h>
>#include <stdlib.h>

Good: <stdlib.his included.

>typedef struct pxl {
double lon, lat;
double x,y,z;
double px,py;
} pixel;

(If you are going to use the "typedef" keyword at all, I prefer to
separate it out. See <http://web.torek.net/torek/c/types2.html>.
Note that this is purely a style issue; there is nothing incorrect
with the code above. It may affect the rest of your structures,
though.)

>struct chn {
int index;
struct nde *node;
};
>
>struct nde {
pixel position;
struct chn *forward;
struct chn *reverse;
int *chain_num;
int size;
};

Note that a "struct nde"'s "forward" and "reverse" elements have
type "struct chn *", i.e., pointer to "struct chn".

>typedef struct chn chain;
>typedef struct nde node;
>
>node *createnode( );

Better to write "node *createnode(voi d);". This is purely a style
issue -- the code is right either way -- but using prototypes is
wise.

>main()
>{

Better to write "int main(void)". (Not *wrong*, at least in C89,
though C99 requires the leading "int" part.)

>
node *startn;
>
startn = createnode( );
>
startn->size = 2;
startn->forward = malloc( 2 * sizeof( chain * ) );
startn->reverse = malloc( 2 * sizeof( chain * ) );

These two calls to malloc do not have the "c.l.c. Standard Approved
Form". Thus, there is a good chance they are wrong. Are they? Let
me compare them to the "approved method" versions, for which the
second line would read:

startn->reverse = malloc(2 * sizeof *startn->reverse);

In both cases, we have "N * sizeof..." where N is 2. So far, so
good -- we want two elements so that we can do:

startn->reverse[0] = some_val;
startn->reverse[1] = another_val;

-- and assuming the malloc() works and the sizes are right, we
should get that. Are the sizes right?

The "non-clc version" uses sizeof(chain *). Here "chain" is a
typedef-alias for "struct chn", so this asks for sizeof(struct
chn *): the size, in C bytes, of a "pointer to struct chn".

For numeric-concreteness purposes let me assume you are using a
typical IA32 system, in which sizeof(struct chn *) is 4, and
sizeof(struct chn) is 8. (If you were on an IA64 system, the sizes
would be 8 and 16, or possibly 8 and 12, respectively. Other
machines may have yet other sizes, but this is probably enough in
the way of examples.) So this asks for 2*4 bytes -- 8 bytes.

The "clc version", on the other hand, uses sizeof(*startn->reverse).
Now, startn->reverse has type "pointer to struct chn", so applying
a unary "*" operator to follow that pointer would produce an object
of type "struct chn". Hence, this is the as sizeof(struct chn),
which -- using the assumptions above -- is 8. So this asks for
2*8, or 16, bytes.

In other words, the sizes are *not* right. This is certainly *a*
problem, if not "the" (entire) problem.

printf("sf %p\nsr %p\n\n",
startn->forward,
startn->reverse
);

Technically you need to convert the two pointer arguments to
"void *" here, although it will work on both IA32 and IA64 example
architectures. In fact, there are almost no machines today on
which the cast changes the underlying machine code. But "almost
no machines" is slightly different from "no machines": if you ever
find yourself compiling on a Data General Eclipse, the code would
behave oddly without such a conversion. It is safer (as in,
guaranteed to be 100% correct, instead of almost-always-works-
but-not-guaranteed) to do, e.g.:

printf("sf %p\nsr %p\n\n",
(void *)startn->forward,
(void *)startn->reverse);

[some snippage]

>node *createnode( )
{
node *node;
node = malloc( sizeof(node) );
return node;
}

Others have pointed out the danger of having too many things named
"node" in the same name-space. (Cf. the "party at which everyone is
named Chris" in the web page I referred to above. In part because
"struct"s have their own name-space, I prefer not to use typedefs
at all.) In this case, that, plus avoiding the "clc-approved form",
result in:

malloc(sizeof node)

asking for sizeof <the variablebytes. On IA32, sizeof <the
variableis 4, while sizeof(struct nde) is 72 -- so this asks for
4 bytes instead of 72. Even with the "everyone-has-the-same-name"
confusion, though, if you had written:

node = malloc(sizeof *node);

this would have asked for sizeof <*(the variable)bytes, and since
the variable has type "pointer to struct nde", and the unary "*"
removes the "pointer to" part, you would have asked for
sizeof(struct nde) bytes -- assuming IA32 again, the 72 you need
here.
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) +1 801 277 2603
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.

[Keith Thompson]

Re: malloc and functions

raphfrk <raphfrk@netsca pe.netwrites:

On Jul 15, 4:34 pm, Robert Gamble <rgambl...@gmai l.comwrote:

>On Jul 15, 10:57 am, raphfrk <raph...@netsca pe.netwrote:
>>

node = malloc( sizeof(node) );

>>
>Hint: Does sizeof apply to the typedef name or the variable name?
>It's probably not what you expect.
>You should also check the return value of malloc for failure.

>
This is the main problem. I went through the original program and
checked every malloc assignment. I didn't get the sizeof argument
correct in a few other places.
>
I had one for an array of the form:
>
char *array;
>
array = malloc( 100*sizeof( char * ) )

[...]

That's why we here in comp.lang.c recommend a consistent idiomatic
form for malloc calls. For the example above, it would be:

char *array;

array = malloc(100 * sizeof *array);

(or, better yet, you'd use a defined constant rather than the magic
number 100).

By using the name of the target pointer object in the sizeof
expression, you can write a correct expression that will remain
correct even if you change (or forget) the type of the pointer.

Incidentally, I probably wouldn't use the name 'array' for a pointer
object. It points to (the first element of) an array, but it isn't an
array itself. In generic or sample code, I'd probably call it
something like "ptr". In real-world application code, I'd give it a
name that reflects what it's actually used for rather than it's C
type.

--
Keith Thompson (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[raphfrk]

Re: malloc and functions

On Jul 15, 3:38 pm, Chris Torek <nos...@torek.n etwrote:

In article <1184511435.763 565.302...@o61g 2000hsh.googleg roups.com>
>
raphfrk <raph...@netsca pe.netwrote:

I am having an issue with malloc and gcc.

>
Issues with malloc() quite often boil down to one or more of
three things:
>
- Failure to include <stdlib.h>. This one is short and mostly
self-explanatory.

Why doesn't this trigger a warning ? In fact, how does it know
what malloc is supposed to return ?

>
- Not using the "comp.lang. c Standard Approved Method" of
calling malloc() :-) The "approved method" says that if
you a pointer variable "p" of type "T *", i.e.:
>
T *p;
>
the call to malloc() should have the form:
>
p = malloc(N * sizeof *p);
>

Good plan, I will change my malloc calls to that.

sizeof isn't a function ?

I assume

p = malloc( N * sizeof( *p ) );

is just as good.