Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > malloc and functions

Reply
Thread Tools

malloc and functions

 
 
Thad Smith
Guest
Posts: n/a
 
      07-15-2007
rzed wrote:
> raphfrk <(E-Mail Removed)> wrote in
> news:(E-Mail Removed) oups.com:
>
>>On Jul 15, 4:09 pm, Richard <(E-Mail Removed)> wrote:
>>
>>>raphfrk <(E-Mail Removed)> writes:
>>>
>>>>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
 
Reply With Quote
 
 
 
 
raphfrk
Guest
Posts: n/a
 
      07-15-2007
On Jul 15, 4:34 pm, Robert Gamble <(E-Mail Removed)> wrote:
> On Jul 15, 10:57 am, raphfrk <(E-Mail Removed)> wrote:
>
> > 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.

 
Reply With Quote
 
 
 
 
Chris Torek
Guest
Posts: n/a
 
      07-15-2007
In article <(E-Mail Removed) .com>
raphfrk <(E-Mail Removed)> wrote:
>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(original) + 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_string) 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.h> is 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(void);". 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 variable> bytes. On IA32, sizeof <the
variable> is 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 (4039.22'N, 11150.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.
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      07-15-2007
raphfrk <(E-Mail Removed)> writes:
> On Jul 15, 4:34 pm, Robert Gamble <(E-Mail Removed)> wrote:
>> On Jul 15, 10:57 am, raphfrk <(E-Mail Removed)> wrote:
>>
>> > 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_Keith) http://www.velocityreviews.com/forums/(E-Mail Removed) <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"
 
Reply With Quote
 
raphfrk
Guest
Posts: n/a
 
      07-16-2007
On Jul 15, 3:38 pm, Chris Torek <(E-Mail Removed)> wrote:
> In article <(E-Mail Removed) .com>
>
> raphfrk <(E-Mail Removed)> wrote:
> >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.

 
Reply With Quote
 
Martin Ambuhl
Guest
Posts: n/a
 
      07-16-2007
raphfrk wrote:
> On Jul 15, 3:38 pm, Chris Torek <(E-Mail Removed)> wrote:
>> In article <(E-Mail Removed) .com>
>>
>> raphfrk <(E-Mail Removed)> wrote:
>>> 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 ?


for almost all compilers I know, it does, if you haven't told the
compiler to shut up by (a) casting the return value from malloc or (b)
setting your diagnostics at a pathetically low level. Both are serious
programmer errors, but only by fixing the programmer can they be corrected.

>> - 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.


There is no difference, except the form 'sizeof *p' makes it clear that
(a) sizeof is not a function and (b) *p is an object. Those things are
obvious, but it is amazing what 'obvious' things are not so to
subsequent code maintainers.

 
Reply With Quote
 
Flash Gordon
Guest
Posts: n/a
 
      07-16-2007
raphfrk wrote, On 16/07/07 17:44:
> On Jul 15, 3:38 pm, Chris Torek <(E-Mail Removed)> wrote:
>> In article <(E-Mail Removed) .com>
>>
>> raphfrk <(E-Mail Removed)> wrote:
>>> 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 ?


If you don't cast the result of malloc then the compiler *will* generate
a diagnostic (warning or error). If you use a cast then you are telling
the compiler you know what you are doing, but *some* compilers will be
helpful and bitch at you anyway.

> In fact, how does it know
> what malloc is supposed to return ?


If you include stdlib.h it knows because stdlib.h tell it, if nothing
tells it the compiler is *required* to assume malloc returns an int
which is incorrect.

>> - 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 ?


No, it is an operator, just like +.

> I assume
>
> p = malloc( N * sizeof( *p ) );
>
> is just as good.


The extra parenthesis are not required but do no harm. Just extra to type!
--
Flash Gordon
 
Reply With Quote
 
Eric Sosman
Guest
Posts: n/a
 
      07-16-2007
raphfrk wrote On 07/16/07 12:44,:
> On Jul 15, 3:38 pm, Chris Torek <(E-Mail Removed)> wrote:
>
>>In article <(E-Mail Removed) .com>
>>
>>raphfrk <(E-Mail Removed)> wrote:
>>
>>>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 ?


On many compilers it does (or can be made to with
suitable compiler options), and on C99 compilers it
must, always.

> In fact, how does it know
> what malloc is supposed to return ?


It doesn't, and that's the problem. If a function
is used without being declared, a C90 compiler assumes
that the function returns an int. Since malloc returns
something that isn't an int, the code the compiler uses
to retrieve the function value may be incorrect. For
example, it might look in accumulator AC0 for an int,
whereas pointer-valued functions place return their
values in index register IX0. Or it might look at just
one half of a register pair, or some other such mismatch.

Under C99 rules, it is an error to use a function
without declaring it first, so the compiler must produce
a diagnostic message. After that, may or may not continue
to try to compile the program.

>> - 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 ?


No; sizeof is an operator.

> I assume
>
> p = malloc( N * sizeof( *p ) );
>
> is just as good.


Yes. The sizeof operator can be used in two ways:

sizeof _expression_
or
sizeof ( _typename_ )

Since an _expression_ surrounded by parentheses is still
an _expression_, the two forms can be made to look alike.
Using the optional parentheses has two important effects:
(1) it may make the code easier to read by making it clear
which things are the operand of sizeof and which are not,
and ((2)) it will make c.l.c. people treat you as if you
were ignorant, because they know something they imagine
you do not.

--
(E-Mail Removed)

 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      07-16-2007
raphfrk <(E-Mail Removed)> writes:

> sizeof isn't a function ?


No, and it is odd even for an operator:

#include <stdio.h>

int f(void) { return printf("In f()\n"); }

int main(void)
{
printf("%zd\n", sizeof f());
return 0;
}

--
Ben.
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      07-17-2007
Ben Bacarisse <(E-Mail Removed)> writes:
> raphfrk <(E-Mail Removed)> writes:
>
>> sizeof isn't a function ?

>
> No, and it is odd even for an operator:
>
> #include <stdio.h>
>
> int f(void) { return printf("In f()\n"); }
>
> int main(void)
> {
> printf("%zd\n", sizeof f());
> return 0;
> }


The "?:", "&&", and "||" operators also don't necessarily evaluate all
their operands.

I think the main reason for the confusion is that sizeof is the only
operator whose symbol is a identifier (specifically a keyword). The
fact that its operand either must be (for a type name) or can be (for
an expression) enclosed in parentheses just adds to the frivolity.

If C had used '$' rather than 'sizeof', nobody would think it's a
function -- but then there would have been problems in C's early
years, when keyboards with no '$' character were widespread.

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <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"
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
to malloc or not to malloc?? Johs32 C Programming 4 03-30-2006 10:03 AM
porting non-malloc code to malloc micromysore@gmail.com C Programming 3 02-19-2005 05:39 AM
Malloc/Free - freeing memory allocated by malloc Peter C Programming 34 10-22-2004 10:23 AM
free'ing malloc'd structure with malloc'd members John C Programming 13 08-02-2004 11:45 AM
Re: free'ing malloc'd structure with malloc'd members ravi C Programming 0 07-30-2004 12:42 PM



Advertisments