Structure having pointers

[Richard]

But no integer is copied: a structure is copied, and a structure cannot have
a trap representation. This might have been different in previous
standards, but currently, you are allowed to copy even completely
uninitialised structures and unions.

This makes more sense to me. The structure "copy/assign" would normally
just be a memcpy or similar I would have thought.

 

[regis]

Thanks for that correction. I'll keep that in mind.

That was my line...

 

[regis]

Is this really defined as Undefined Behaviour?

I have seen TONS of SW where structures only set certain elements and
then are shallow copied for inclusion into linked lists etc.

Thinking of it, this is just how changing the attributes
of X11 window work: we partially fill a structure
with the values we want to change and we set a bit-mask
indicating which values are defined in the structure...

 

[regis]

Thinking of it, this is just how changing the attributes
of X11 window work: we partially fill a structure
with the values we want to change and we set a bit-mask
indicating which values are defined in the structure...

time to put some distance between me and from keyboard,
the structure is not passed by value to the function and
hence a copy the partially filled struct is not done...

 

[Richard]

As Harald has pointed out, I'm wrong on this.

I posted that before Harald replied. Pete was too. Thanks for
acknowledging.

 

[Richard]

Thinking of it, this is just how changing the attributes
of X11 window work: we partially fill a structure
with the values we want to change and we set a bit-mask
indicating which values are defined in the structure...

But for totally different reasons I suspect. At a guess this is so that
you only send changed/set parameters in the X protocol - reduces
bandwidth requirements. Nothing to do with "undefined behaviour invoked
by copying an uninitialized integer".

Besides, I seriously doubt that the integer field of a newly created
structure can ever hold an "illegal" system value in the real world.

 

[Kenny McCormack]

Besides, I seriously doubt that the integer field of a newly created
structure can ever hold an "illegal" system value in the real world.

The real world is (by explicit decree - no, I am not making this up)
irrelevant here.

 

[Malcolm McLean]

Besides, I seriously doubt that the integer field of a newly created
structure can ever hold an "illegal" system value in the real world.

It's a potential enhancement. All unitialised objects are set to trap. If
the value is used then the program aborts.
I don't know of a single system that actually does this, but it would be
better, since you'd catch bugs earlier. The downside is that a struct
assignment would need care, because logically you might not need certain
fields, but the compiler can't be expected to know that.

 

[Richard]

It's a potential enhancement. All unitialised objects are set to
trap. If the value is used then the program aborts.
I don't know of a single system that actually does this, but it would
be better, since you'd catch bugs earlier. The downside is that a
struct assignment would need care, because logically you might not
need certain fields, but the compiler can't be expected to know that.

It's a terrible idea. I don't necessarily WANT to initialise all
variables in certain structures. A control flag dictates which fields
are in "use" and don't all necessarily lend themselves to being in a Union.

If you want that level of control use something like ADA. C is C for a
reason - powerful and efficient and puts things in the programmers hands.

 

[Kenny McCormack]

It's a terrible idea. I don't necessarily WANT to initialise all
variables in certain structures. A control flag dictates which fields
are in "use" and don't all necessarily lend themselves to being in a Union.

It's a really terrible idea because it would break tons of existing, working,
code.

 

[Malcolm McLean]

struct Sample *sample_clone(struct Sample *sp)
{
struct Sample *answer = malloc(sizeof *answer);
if (answer && (answer->p = malloc(sp->i)))
memcpy(answer->p, sp->p, sp->i);
else {
free(answer);
answer = NULL;
}
return answer;
}

This approach is a blind alley. As you add members to logic to free and
abort becomes more and more complicated. It is a lot better to have a "clean
up and return" section that is quite clearly not part of normal execution.
Poor man's exception handling, if you like.

void sample_kill(struct Sample *sp)
{
if (sp) free(sp->p);
free(sp);
}

I know that many people don't like this style, but I think C works
that way. I am sure that it is not accidental that things like short
circuit &&, assignments in conditionals, and being able to free NULL all
work together to make compact, idiomatic code.

The problem isthat for such a simple structure you are right. Add another
array and you've got to have a curly brace anyway. Get into the habit of

void killme(MYSTRUCT *ptr)
{
if(ptr)
{
}
}
Then you drop fewer stitches. It's a completely boilerplate approach, no-one
has to think, which is right because this is just boring code. The algorithm
is elsewhere.

 

[pete]

But no integer is copied: a structure is copied,
and a structure cannot have
a trap representation. This might have been different in previous
standards, but currently, you are allowed to copy even completely
uninitialised structures and unions.

The C90 standard doesn't have the word "trap" in it,
but it does say this about indeterminate objects:

3.16 undefined behavior:
Behavior, upon use of a nonportable or erroneous program
construct, of erroneous data,
or of indeterminately valued objects,
for which this International Standard imposes no requirements.

Where in the C99 standard does it say no traps for structs?

All that I can find about traps and structs, applies to padding.

 

[Keith Thompson]

AnticitizenOne said: [...]
why in C is not recommended?

Several reasons, but before we get into those reasons, let's find out
your reasons for using the cast.

I use the cast as referenced in C-Language (Kerningan & Ritchie)

That book was written before the first Standard for C was published. At that
point, there was no void * as a generic pointer type, but instead that
purpose was served by the char * type. Since then it's not necessary in C
to cast between a void * and another pointer type. Any pointer can be
converted to a void * and back again without any loss of information.

I suspect the OP is using the second edition of Kernighan & Ritchie,
not K&R1. K&R2 was published shortly before the first C standard, but
describes essentially the same language as the C89 standard does --
including void*. As I recall, the examples in K&R2 do cast the result
of malloc, but the errata list at
<http://cm.bell-labs.com/cm/cs/cbook/2ediffs.html> says:

The remark about casting the return value of malloc ("the proper
method is to declare ... then explicitly coerce") needs to be
rewritten. The example is correct and works, but the advice is
debatable in the context of the 1988-1989 ANSI/ISO standards. It's
not necessary (given that coercion of void * to ALMOSTANYTYPE * is
automatic), and possibly harmful if malloc, or a proxy for it,
fails to be declared as returning void *. The explicit cast can
cover up an unintended error. On the other hand, pre-ANSI, the
cast was necessary, and it is in C++ also.

If the OP really is using the first edition, he should get a copy of
the second edition; K&R1 is historically interesting, and it's still a
good book, but K&R2 is much more up to date.

See also questions 7.6, 7.7, 7.7b, and 7.7c in the comp.lang.c FAQ,
<http://www.c-faq.com/>.

 

[Keith Thompson]

The keyword is struct, not Struct. C is case sensitive.


The declaration should be:

struct Sample a;


As above.


Since a.i has an indeterminate value, the copy invokes undefined behaviour.

[...]

This is actually ambiguous in both C90 and C99. n1124 (which
incorporates C99 plus Technical Corrigenda 1 and 2) adds the following
wording in 6.2.6.1p6:

The value of a structure or union object is never a trap
representation, even though the value of a member of the structure
or union object may be a trap representation.

This was probably the intent previously, but it hadn't been stated
explicitly. Prior to n1124, it could be argued that the copy does
invoke UB. But I doubt that any real-world implementation does
anything other than just copying the representation using the
equivalent of memcpy(). Even an implementation that copies the
individual members one by one would be unlikely to cause problems.

 

[Keith Thompson]

Where in the C99 standard does it say no traps for structs?

It doesn't, but n1124 does. See my response elsethread.

 

[Ben Bacarisse]

This approach is a blind alley. As you add members to logic to free
and abort becomes more and more complicated.

That arguments sounds like "it is going to get messy do lets start
messy". I'd prefer to solve today's problem and re-organise[1] *if*
required tomorrow -- the design version of avoiding premature
optimisation. However...

It is a lot better to
have a "clean up and return" section that is quite clearly not part of
normal execution.

In fact, that is pretty much what I have. It is the else. I think the
key thing is to ensure that all pointers are either NULL or malloc'ed
at the key times. calloc is not required to return storage
initialised to something that compares == to a null pointer constant,
so a bit more work is needed when the space has more than one pointer,
but it scales to more than one just fine:

struct big_sample *bsp = malloc(sizeof *bsp);
if (bsp) {
*bsp = (struct big_sample){0};
if ((bsp->ptr1 = malloc(src->sz1)) &&
(bsp->ptr2 = malloc(src->sz2)) &&
(bsp->ptr3 = malloc(src->sz3)) &&
(bsp->ptr4 = malloc(src->sz4))) {
/* all good to go now */
memcpy(bsp->ptr1, src->ptr1, src->sz1);
memcpy(bsp->ptr2, src->ptr2, src->sz2);
memcpy(bsp->ptr3, src->ptr3, src->sz3);
memcpy(bsp->ptr4, src->ptr4, src->sz4);
}
else {
free(bsp->ptr1);
free(bsp->ptr2);
free(bsp->ptr3);
free(bsp->ptr4);
free(bsp);
bsp = NULL;
}
}
return bsp;

so you get a bump from one if to two when you go from the common case
of one dependent thing to more than one, but it scales just fine.
Obviously, with this much regularity one would be using an array -- I
just kept the pattern simple so you could see how it scales. In a
real messy case, all the mallocs, sizes and memcpys would be different
and quirky.

The problem isthat for such a simple structure you are right. Add
another array and you've got to have a curly brace anyway. Get into
the habit of

void killme(MYSTRUCT *ptr)
{
if(ptr)
{
}
}

It is a matter of taste to some extent. If that were the "house
style" I'd use it, but I prefer minimal braces myself. I can't see:

if (sp)
free(sp->p1);
free(sp->p2);
free(sp);

(which is how the error would look) without a big alarm going off --
I'm getting a funny feeling just letting it sit there on the screen
now. For me, C's { }s are HUGE -- maybe from learning LISP so long
ago.

 

[Ben Bacarisse]

I'd prefer to solve today's problem and re-organise[1] *if*
required tomorrow -- the design version of avoiding premature
optimisation.

I forgot my footnote:
[1] I am not keen on "refactor" for programs. It's just a linguistic
thing, the idea is fine.

 

[Malcolm McLean]

This approach is a blind alley. As you add members to logic to free
and abort becomes more and more complicated.

That arguments sounds like "it is going to get messy do lets start
messy". I'd prefer to solve today's problem and re-organise[1] *if*
required tomorrow -- the design version of avoiding premature
optimisation. However...

It is a lot better to
have a "clean up and return" section that is quite clearly not part of
normal execution.

In fact, that is pretty much what I have. It is the else. I think the
key thing is to ensure that all pointers are either NULL or malloc'ed
at the key times. calloc is not required to return storage
initialised to something that compares == to a null pointer constant,
so a bit more work is needed when the space has more than one pointer,
but it scales to more than one just fine:

struct big_sample *bsp = malloc(sizeof *bsp);
if (bsp) {
*bsp = (struct big_sample){0};
if ((bsp->ptr1 = malloc(src->sz1)) &&
(bsp->ptr2 = malloc(src->sz2)) &&
(bsp->ptr3 = malloc(src->sz3)) &&
(bsp->ptr4 = malloc(src->sz4))) {
/* all good to go now */
memcpy(bsp->ptr1, src->ptr1, src->sz1);
memcpy(bsp->ptr2, src->ptr2, src->sz2);
memcpy(bsp->ptr3, src->ptr3, src->sz3);
memcpy(bsp->ptr4, src->ptr4, src->sz4);
}
else {
free(bsp->ptr1);
free(bsp->ptr2);
free(bsp->ptr3);
free(bsp->ptr4);
free(bsp);
bsp = NULL;
}
}
return bsp;

so you get a bump from one if to two when you go from the common case
of one dependent thing to more than one, but it scales just fine.
Obviously, with this much regularity one would be using an array -- I
just kept the pattern simple so you could see how it scales. In a
real messy case, all the mallocs, sizes and memcpys would be different
and quirky.

The problem isthat for such a simple structure you are right. Add
another array and you've got to have a curly brace anyway. Get into
the habit of

void killme(MYSTRUCT *ptr)
{
if(ptr)
{
}
}

It is a matter of taste to some extent. If that were the "house
style" I'd use it, but I prefer minimal braces myself. I can't see:

if (sp)
free(sp->p1);
free(sp->p2);
free(sp);

(which is how the error would look) without a big alarm going off --
I'm getting a funny feeling just letting it sit there on the screen
now. For me, C's { }s are HUGE -- maybe from learning LISP so long
ago.

Your code looks like Lisp.
I'm sure it's possible to write an entire program in one if() statement,
with & and || providing the flow control, and recursuive calls to give
generality.
But it isn't C.
Your code seems to be desperate attempt to avoid a goto. Why? Just because
some person once published a paper?

 

[Ben Bacarisse]

Your code looks like Lisp.

You need to look a some LISP again! It does not even violate your
rather arbitrary "no more than three levels of brackets" rule that you
posted a while ago.

I'm sure it's possible to write an entire program in one if()
statement, with & and || providing the flow control, and recursuive
calls to give generality.
But it isn't C.

You do like to fight both sides of the debate! I don't know if what
you suggest is possible, but since I did not do it, I don't see the
value in bringing up such an absurd idea.

Your code seems to be desperate attempt to avoid a goto. Why? Just
because some person once published a paper?

No. I want the conditions under which code is executed to be clear
and self-evident. It is safe to do the object copy if (and only if)
all the storage has been allocated; and that should be stated as
clearly as possible. I think an 'if' does that. There are lots of
ways of calculating the expression used in the 'if'. Local house
style rules might require:


bsp->ptr1 = malloc(src->sz1);
bsp->ptr2 = malloc(src->sz2);
bsp->ptr3 = malloc(src->sz3);
bsp->ptr4 = malloc(src->sz4);
if (bsp->ptr1 && bsp->ptr2 && bsp->ptr3 && bsp->ptr4) {
...
}

or even:

int n_errors = 0;
if (bsp->ptr1 = malloc(src->sz1)) == NULL)
n_errors += 1;
...
if (n_errors == 0) {
...
}

I just put the code in to the condition explicitly -- the pattern is
the same.

Please note, I was never claiming good style -- that is a very complex
topic involving lots of variables, many of them social rather than
technical. I was refuting the bold statement that my approach was "a
blind alley"[1].

[1] I would say "dead end". Blind alleys can be fun to follow and
often lead to wide, appealing, vistas.

 

[Chris Dollin]

[1] I would say "dead end". Blind alleys can be fun to follow and
often lead to wide, appealing, vistas.

(fx:OT) A blind alley /is/ a dead end. If you can get through it
to somewhere else, it isn't blind.