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