compiling on different compilers

[MJL]

I am working on a set of functions that involve dynamic memory
allocation. I started working with gcc and then moved to Pacific C.
The reason was that my only access to gcc was through an ssh
connection that I did not have access to all the time, so I got
Pacific so I could work on my home windows computer.

Then I moved back to gcc, which saved me from some major mistakes.
Somehow Pacific C was letting me get away with stuff that was
completely wrong. gcc immediately gave me segmentation errors until I
got every last thing correct.

Is gcc the best for catching mistakes? If not, which one is?

 

[Emmanuel Delahaye]

MJL wrote on 12/08/04 :

I am working on a set of functions that involve dynamic memory
allocation. I started working with gcc and then moved to Pacific C.
The reason was that my only access to gcc was through an ssh
connection that I did not have access to all the time, so I got
Pacific so I could work on my home windows computer.

Assuming Windows, you can do the same at home. gcc is a very common C
compiler used in DJGPP, Mingw, Dev-C++ etc. or even standalone at
command line.

Then I moved back to gcc, which saved me from some major mistakes.
Somehow Pacific C was letting me get away with stuff that was
completely wrong. gcc immediately gave me segmentation errors until I
got every last thing correct.

Is gcc the best for catching mistakes? If not, which one is?

It's good, but PCLint (test only, no compile) goes further. Note that
tools don't catch all mistakes. There is nothing like a review by an
experienced programmer (like you [are]/[will be]).

 

[CBFalconer]

MJL wrote on 12/08/04 :

.... snip ...

Is gcc the best for catching mistakes? If not, which one is?

It's good, but PCLint (test only, no compile) goes further. Note
that tools don't catch all mistakes. There is nothing like a
review by an experienced programmer (like you [are]/[will be]).

splint (nee lclint) is also good, but limited to C (no C++). It
is also considerably cheaper.

 

[MJL]

Thanks for the info.

I am writing a set of functions which operate on a linked list of a
struct which holds a string. I started working on it in the interest
of maximum portability of my code. I have found since that I really
enjoy working with char* strings, linked lists, malloc etc...

My biggest mistake that slipped by that other compiler was something
like:

mystruct* pm;

pm = (mystruct*)malloc(sizeof(mystruct*));

which should have been:

pm = (mystruct*)malloc(sizeof(mystruct));

I was allocating memory for the size of the pointer and not the actual
struct.

also:

void init(mystruct* pm)
{
pm = (mystruct*)malloc(sizeof(mystruct));
pm->first = NULL;
pm->second = NULL;
}

Here I was trying to change what pm pointed to, not the contents of
the memory being pointed to. When the function returned, the pointer
pm still pointed to whatever it was pointing to before the function
call.

For small examples, the one compiler was letting me get away with my
mistakes. gcc caused my program to give the segmentation fault error
every time until I fixed all mistakes. Thanks again.

mjl

 

[Emmanuel Delahaye]

MJL wrote on 12/08/04 :

My biggest mistake that slipped by that other compiler was something
like:

mystruct* pm;

pm = (mystruct*)malloc(sizeof(mystruct*));

which should have been:

pm = (mystruct*)malloc(sizeof(mystruct));

You should try another programming style, like the one recommended on
this forum:

mystruct* pm = malloc (sizeof *pm);

because it's exactly what you want.

 

[CBFalconer]

.... snip (handled by ED) ...

void init(mystruct* pm)
{
pm = (mystruct*)malloc(sizeof(mystruct));
pm->first = NULL;
pm->second = NULL;
}

Here I was trying to change what pm pointed to, not the contents
of the memory being pointed to. When the function returned, the
pointer pm still pointed to whatever it was pointing to before
the function call.

For small examples, the one compiler was letting me get away with
my mistakes. gcc caused my program to give the segmentation fault
error every time until I fixed all mistakes. Thanks again.

With gcc you should use "-W -Wall -ansi -pedantic" when developing
portable code, i.e. most of the time.

Here you have the same error as pointed out by ED before, and also
have developed a memory leak. You have received a pointer from
malloc, and discarded it when returning. Try:

mystruct* createmystruct(void)
{
mystruct *pm;

if (pm = malloc(sizeof *pm)) {
pm->first = pm->second = NULL;
}
return pm;
}

 

[MJL]

pm = (mystruct*)malloc(sizeof(mystruct));

You should try another programming style, like the one recommended on
this forum:

mystruct* pm = malloc (sizeof *pm);

because it's exactly what you want.

hmmm...

Is my version an error or just not the recommended style? I may be
wrong, but the two versions seem equivalent.

mystruct* pm = (mystruct*)malloc(sizeof(mystruct));

pointer of type mystruct* is being pointed to an allocation of enough
memory for a structure of type mystruct.

compare to:

mystruct* pm = malloc (sizeof *pm);

pointer of type mystruct* is being pointed to an allocation of enough
memory for the structure that pm points to (which is a mystruct).

I accept the second version if this is standard, but it gives me an
uneasy feeling of being a circular argument. I based my version on
the string version which I have seen in print I think:

char* str = (char*)malloc(sizeof(char))

I was just wondering if my version is incorrect and will cause errors,
because so far it is working ok. I will of course, for clarity of my
code and compliance with standard practices, change to the accepted
version regardless.

-mjl

 

[Emmanuel Delahaye]

MJL wrote on 13/08/04 :

Is my version an error or just not the recommended style? I may be
wrong, but the two versions seem equivalent.

They have the same behaviour. Just one is much simpler to use and
maintain than the other. The choice is up to you.

 

[Jens.Toerring]

Is my version an error or just not the recommended style? I may be
wrong, but the two versions seem equivalent.

mystruct* pm = (mystruct*)malloc(sizeof(mystruct));

pointer of type mystruct* is being pointed to an allocation of enough
memory for a structure of type mystruct.

compare to:

mystruct* pm = malloc (sizeof *pm);

pointer of type mystruct* is being pointed to an allocation of enough
memory for the structure that pm points to (which is a mystruct).

I accept the second version if this is standard, but it gives me an
uneasy feeling of being a circular argument. I based my version on

Luckily, there isn't anything circular - when you initialize 'pm'
(e.g. by calling malloc()) the compiler already knows what 'pm'
and, more important, what its type is.

the string version which I have seen in print I think:

char* str = (char*)malloc(sizeof(char))

I was just wondering if my version is incorrect and will cause errors,
because so far it is working ok. I will of course, for clarity of my
code and compliance with standard practices, change to the accepted
version regardless.

No, your version is not wrong. You can cast the return value of
malloc() and you can use sizeof(type) without any problems at all.
But most regulars here (with the notable exception of Mr. Plauger,
but who, as far as I understand it, is more concerned with code
that compiles cleanly with both a C and and a C++ compiler) feel
that casting malloc()'s return value isn't to be recommended
because it does not buy you anything (except acceptance by a C++
compiler) but can keep the compiler from warning you if you have
forgotten to include <stdlib.h> - an argument that seems to have
even convinced K&R since in their errata page for K&R2 they
retract their previously recommmendation of using the cast.

In the same sense using e.g. "sizeof(mystruct)" is absolutely
correct. But again there seems to be some kind of consensus here
that there are advantages of using the other form where not the
type but the name of the variable is used. The argument for this
is mostly about the fact that when you use the type you have to
replace it everywhere in your code if the name gets changed,
which, according to Murphy's law, will result in you overlooking
it in the place where it can do the maximum damage. Using the name
of the variable can help you avoid that since you have to change
the word "mystruct" only at the places where you define variables
of that type (which, at least in C89, tends to be mostly near the
start of functions) and you have a better chance of finding all
instances where you have to change them or at least less places
where you can make mistakes. While that may seem to be a small
benefit when you have a 100 lines program it can make quite a
bit of a differences when you have to go through several dozends
of source files, each with several hundreds or more lines. And,
of course, an even more convincing argument is lazyness;-)

Regards, Jens

 

[Eric Sosman]

[ concerning malloc() style ]
In the same sense using e.g. "sizeof(mystruct)" is absolutely
correct. But again there seems to be some kind of consensus here
that there are advantages of using the other form where not the
type but the name of the variable is used. The argument for this
is mostly about the fact that when you use the type you have to
replace it everywhere in your code if the name gets changed,
which, according to Murphy's law, will result in you overlooking
it in the place where it can do the maximum damage. [...]

IMHO that's really not a particularly compelling
reason. A much stronger reason is that the recommended
style avoids exactly the error the O.P. encountered:

> mystruct* pm;
> pm = (mystruct*)malloc(sizeof(mystruct*));

.... where the byte count given to malloc() is (probably)
incorrect.

A possibly more common variant of this error crops
up in programs that have several different struct types
floating around:

typedef struct { ... } PacketHeader;
typedef struct { ... } MessageHeader;
typedef struct { ... } ImageHeader;
typedef struct { ... } FileHeader;
typedef struct { ... } GratefulDeader;
...
MessageHeader *p;
...
p = malloc(sizeof(PacketHeader));

Again, the `malloc(sizeof *p)' style makes this kind of
mistake impossible. That, I think, is the principal reason
to prefer it.

 

[Jarno A Wuolijoki]

A possibly more common variant of this error crops
up in programs that have several different struct types
floating around:

typedef struct { ... } PacketHeader;
typedef struct { ... } MessageHeader;
typedef struct { ... } ImageHeader;
typedef struct { ... } FileHeader;
typedef struct { ... } GratefulDeader;
...
MessageHeader *p;
...
p = malloc(sizeof(PacketHeader));

Again, the `malloc(sizeof *p)' style makes this kind of
mistake impossible. That, I think, is the principal reason
to prefer it.

PacketHeader *p;
MessageHeader *q;
ImageHeader *r;
FileHeader *s;
GratefulDeader *t;
....
p = malloc(sizeof *p);
q = malloc(sizeof *p);
r = malloc(sizeof *p);
s = malloc(sizeof *p);
t = malloc(sizeof *p);

 

[Eric Sosman]

PacketHeader *p;
MessageHeader *q;
ImageHeader *r;
FileHeader *s;
GratefulDeader *t;
...
p = malloc(sizeof *p);
q = malloc(sizeof *p);
r = malloc(sizeof *p);
s = malloc(sizeof *p);
t = malloc(sizeof *p);

Well, yes -- but only the first of these forms matches
the recommended style. In all the other cases it's easy to
spot the error by inspecting the one line alone, without
scrolling around trying to find the declaration of `q' or
`r' or whatever. If you see

ptr = malloc(sizeof *ptr);

(and if the compiler does not complain that `ptr' is not a
pointer), you know at once that the correct amount of
memory has been requested. On the other hand, if you see

ptr = malloc(sizeof(BucketOfBits));

you cannot tell whether it's right or wrong without hunting
up the actual declaration of `ptr'. The recommended style
localizes all that's needed for a correctness proof.

Of course, it's still possible to make errors in memory
allocation, even with the recommended form!

ptr = malloc(n * sizeof *ptr);
if (ptr != NULL) {
while (n >= 0)
ptr[n--] = expression;
}

will not be caught by the suggested style. Seat belts save
lives, but not every life in every crash.

 

[J. J. Farrell]

Is my version an error or just not the recommended style? I may be
wrong, but the two versions seem equivalent.

mystruct* pm = (mystruct*)malloc(sizeof(mystruct));

compare to:

mystruct* pm = malloc (sizeof *pm);

The two are equivalent, but most people consider your version to
be more error prone, and poor style, for a number of reasons:

- casting the return value from malloc() may hide a bug if
you forget to include the correct header file to declare
malloc(). In this case, the compiler is required to give
you a diagnostic if there is no cast. Many compilers will
give you a warning even if you do have the cast, but there
is no guarantee that they will and you may need to switch
on a particular warning level.

- using <sizeof *pm> makes the code more self-adjusting in
case of change. Suppose you want to change the type of this
storage area to <herstruct>. In your version, you need to
remember to change the type in two places (three if you keep
the cast), and failing to do so will result in a silent bug
that may cause buffer overruns. In the widely preferred
version, you change the type in one place and everything
else adjusts automatically.

- casting the return value from malloc() is simply not necessary.
Having unnecessary stuff in the code makes it harder to
maintain, and more prone to error during maintenance. When I
see your version, I have to work out what the cast is doing
and why it is there. This wastes my time, and makes it more
likely that I will misinterpret what the code does simply
because there is more code for me to interpret.

- it takes more typing for no benefit.

I accept the second version if this is standard, but it gives me an
uneasy feeling of being a circular argument.

Don't be uneasy - that's what's good about it! Everything on
the line that depends on the type of the object depends on the
same mention of the type.

I based my version on
the string version which I have seen in print I think:

char* str = (char*)malloc(sizeof(char))

In that case, I would throw away that bit of print. There are an
awful lot of bad books on C, and even more bad advice on the net.

I was just wondering if my version is incorrect and will cause errors,
because so far it is working ok.

It's correct and will not of itself cause errors as it stands. It
might hide another error (omit the declaration of malloc() and
you'll end up with a run-time error in some 64-bit environments,
for example). It is more prone to error if ever you modify it.

I will of course, for clarity of my
code and compliance with standard practices, change to the accepted
version regardless.

Very wise!

 

[Mark McIntyre]

On 12 Aug 2004 16:51:03 -0700, in comp.lang.c ,

hmmm...

Is my version an error or just not the recommended style? I may be
wrong, but the two versions seem equivalent.

They're technically equivalent, but the CLC_recommended version has two
benefits:
a) the cast in your version is superfluous (in C code), and can even hide
errors such as failing to #include the right header

b) the use of *object over (type) makes your code more maintainable.

 

[Mark McIntyre]

PacketHeader *p;
MessageHeader *q;
...
p = malloc(sizeof *p);
q = malloc(sizeof *p);

But this mistake is much easier to spot, IMHO, since the LHS of the
expression is different to the operand of sizeof. Whereas in the original,
without instant access to the definition of p, you have no way to know
whether the malloc statement is wrong.