Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Structure having pointers

1 view
Skip to first unread message

sam...@yahoo.co.in

unread,
Jul 29, 2007, 6:20:00 AM7/29/07
to
Hi,

I have the following Struct,

Struct Sample
{
int i;
char *p;
};

int main()
{
Sample a;
a.p = malloc(10);
Sample b;
b = a;
}

Now i think a shallow copy is done and if i destry only on the object
there would be a dangling pointer.
How do i overcome this problem as C structures don't support
functions?

Thanks in advance!!!

AnticitizenOne

unread,
Jul 29, 2007, 7:26:11 AM7/29/07
to
Hi,
when you write b=a you copy the memory of the struct instance a in b.
In this way you copy the pointer p of a in b. Now a.p and b.p point to
the same memory.
To make a depth copy of the struct you have to write a function:

struct Sample* SampleCopy(struct Sample* src)
{
struct Sample* b=NULL;
b=(struct Sample*)malloc(sizeof(struct Sample));
b.i=src.i;
b.p=(char*)malloc(sizeof(char)*(strlen(src.p)+1));
memcpy(b.p,src.p);
return b;
}

int main()
{
Sample a;
a.p = (char*)malloc(10);
Sample *b;
b = SampleCopy(&a);

free(a.p);
free(a);
printf("%s",b->p); //should work!!
}

I think in this way you may solve your problem... if I've understood.
Probably I've committed some errors in the code.. but I've not a
reference now!

Bye
Gio

santosh

unread,
Jul 29, 2007, 7:57:13 AM7/29/07
to
sam...@yahoo.co.in wrote:

> Hi,
>
> I have the following Struct,
>
> Struct Sample

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

> {
> int i;
> char *p;
> };
>
> int main()
> {
> Sample a;

The declaration should be:

struct Sample a;

> a.p = malloc(10);
> Sample b;

As above.

> b = a;
> }
>
> Now i think a shallow copy is done

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

> and if i destry only on the object
> there would be a dangling pointer.
> How do i overcome this problem as C structures don't support
> functions?

Deallocate the memory by calling free on either a.p or b.p, and set them
both to NULL.

santosh

unread,
Jul 29, 2007, 8:04:40 AM7/29/07
to
AnticitizenOne wrote:

[top-post corrected]

> On 29 Lug, 12:20, sam_...@yahoo.co.in wrote:
>> Hi,
>>
>> I have the following Struct,
>>
>> Struct Sample
>> {
>> int i;
>> char *p;
>> };
>>
>> int main()
>> {
>> Sample a;
>> a.p = malloc(10);
>> Sample b;
>> b = a;
>>
>> }
>>
>> Now i think a shallow copy is done and if i destry only on the object
>> there would be a dangling pointer.
>> How do i overcome this problem as C structures don't support
>> functions?
>

> Hi,
> when you write b=a you copy the memory of the struct instance a in b.
> In this way you copy the pointer p of a in b. Now a.p and b.p point to
> the same memory.
> To make a depth copy of the struct you have to write a function:
>
> struct Sample* SampleCopy(struct Sample* src)
> {
> struct Sample* b=NULL;
> b=(struct Sample*)malloc(sizeof(struct Sample));

The cast isn't recommended in C.

> b.i=src.i;
> b.p=(char*)malloc(sizeof(char)*(strlen(src.p)+1));

And sizeof(char) is always one in C.

> memcpy(b.p,src.p);
> return b;
> }
>
> int main()
> {
> Sample a;

It should be struct Sample a;

> a.p = (char*)malloc(10);
> Sample *b;

Mixed code and declarations are not allowed in C89 though they're allowed in
C99.

> b = SampleCopy(&a);
>
> free(a.p);
> free(a);
> printf("%s",b->p); //should work!!

return 0;

> }

Malcolm McLean

unread,
Jul 29, 2007, 8:06:17 AM7/29/07
to

<sam...@yahoo.co.in> wrote in message
news:1185704400.2...@i38g2000prf.googlegroups.com...
Do it like this

struct Sample *sample(int N)
{
struct Sample *answer;
answer = malloc(sizeof(struct Sample));
if(!answer)
goto error_exit;
answer.p = malloc(N);
if(!answer.p)
goto error_exit;
return answer;
error_exit:
killSample(answer);
return 0;
}

void killSample(struct Sample *s)
{
if(s)
{
free(s.p);
free(s);
}
}

struct Sample *Sample_clone(struct Sample *s)
{
struct Sample *answer;

answer = sample(s.N):
if(!answer)
return 0;
memcpy(answer.p, s.p, s.N);
return answer;
}

All we are doing is replacing member functions with fucntions which take a
struct Sample * as a parameter. For simplicity everything is kept in dynamic
memory - very rarely this will cause a performance problem and need to be
changed - but it simpifies the code management.

--
Free games and programming goodies.
http://www.personal.leeds.ac.uk/~bgy1mm

Richard Heathfield

unread,
Jul 29, 2007, 8:18:46 AM7/29/07
to
sam...@yahoo.co.in said:

> Hi,
>
> I have the following Struct,
>
> Struct Sample
> {
> int i;
> char *p;
> };
>
> int main()
> {
> Sample a;
> a.p = malloc(10);
> Sample b;
> b = a;
> }

In addition to other people's comments, I would point out that this code
is not legal in either C90 or C99. The absence of a function declarator
for malloc requires a diagnostic message under the rules of either
Standard, and the mixing of declarations with code requires a
diagnostic message under C90.

I suggest you increase your compiler's level of diagnostic checking
until it conforms with the Standard's requirements.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999

AnticitizenOne

unread,
Jul 29, 2007, 8:31:50 AM7/29/07
to

Sorry for my errors... 90% of them are for distraction and cut'n
paste...

> > b=(struct Sample*)malloc(sizeof(struct Sample));
>
> The cast isn't recommended in C.
>

This is really new for me o_O
I use the cast a lot... why in C is not recommended?

thanks!
bye
Gio

Richard Heathfield

unread,
Jul 29, 2007, 8:39:20 AM7/29/07
to
AnticitizenOne said:

> On 29 Lug, 14:04, santosh <santosh....@gmail.com> wrote:
>> AnticitizenOne wrote:

<snip>



>> > b=(struct Sample*)malloc(sizeof(struct Sample));
>>
>> The cast isn't recommended in C.
>>
> This is really new for me o_O
> I use the cast a lot...

Why?

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

Richard

unread,
Jul 29, 2007, 8:31:36 AM7/29/07
to
santosh <santo...@gmail.com> writes:

> sam...@yahoo.co.in wrote:
>
>> Hi,
>>
>> I have the following Struct,
>>
>> Struct Sample
>
> The keyword is struct, not Struct. C is case sensitive.
>
>> {
>> int i;
>> char *p;
>> };
>>
>> int main()
>> {
>> Sample a;
>
> The declaration should be:
>
> struct Sample a;
>
>> a.p = malloc(10);
>> Sample b;
>
> As above.
>
>> b = a;
>> }
>>
>> Now i think a shallow copy is done
>
> Since a.i has an indeterminate value, the copy invokes undefined
> behaviour.

How?

>
>> and if i destry only on the object
>> there would be a dangling pointer.
>> How do i overcome this problem as C structures don't support
>> functions?
>
> Deallocate the memory by calling free on either a.p or b.p, and set them
> both to NULL.
>

--

AnticitizenOne

unread,
Jul 29, 2007, 8:41:38 AM7/29/07
to

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

santosh

unread,
Jul 29, 2007, 8:53:22 AM7/29/07
to
AnticitizenOne wrote:

> On 29 Lug, 14:39, Richard Heathfield <r...@see.sig.invalid> wrote:
>> AnticitizenOne said:
>>
>> > On 29 Lug, 14:04, santosh <santosh....@gmail.com> wrote:
>> >> AnticitizenOne wrote:
>>
>> <snip>
>>
>> >> > b=(struct Sample*)malloc(sizeof(struct Sample));
>>
>> >> The cast isn't recommended in C.
>>
>> > This is really new for me o_O
>> > I use the cast a lot...
>>
>> Why?
>>
>> > 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.

Infact casting the return value of malloc can prevent your compiler from
warning you when you fail to include stdlib.h, which declares malloc's
prototype. This can lead to possible undefined behaviour.

The only real reason for casting the return of malloc in C is if you're
forced to compile it under a C++ compiler or in a mixed C and C++
environment. That's pretty rare.

AnticitizenOne

unread,
Jul 29, 2007, 9:02:30 AM7/29/07
to

uh! Thanks! I've understood.

pete

unread,
Jul 29, 2007, 9:17:31 AM7/29/07
to
Richard wrote:
>
> santosh <santo...@gmail.com> writes:
>
> > sam...@yahoo.co.in wrote:
> >
> >> Hi,
> >>
> >> I have the following Struct,
> >>
> >> Struct Sample
> >
> > The keyword is struct, not Struct. C is case sensitive.
> >
> >> {
> >> int i;
> >> char *p;
> >> };
> >>
> >> int main()
> >> {
> >> Sample a;
> >
> > The declaration should be:
> >
> > struct Sample a;
> >
> >> a.p = malloc(10);
> >> Sample b;
> >
> > As above.
> >
> >> b = a;
> >> }
> >>
> >> Now i think a shallow copy is done
> >
> > Since a.i has an indeterminate value, the copy invokes undefined
> > behaviour.
>
> How?

Simply and obviously.

--
pete

Ben Bacarisse

unread,
Jul 29, 2007, 9:23:07 AM7/29/07
to
"Malcolm McLean" <regn...@btinternet.com> writes:

> <sam...@yahoo.co.in> wrote in message
> news:1185704400.2...@i38g2000prf.googlegroups.com...
>> Hi,
>>
>> I have the following Struct,
>>
>> Struct Sample
>> {
>> int i;
>> char *p;
>> };
>>
>> int main()
>> {
>> Sample a;
>> a.p = malloc(10);
>> Sample b;
>> b = a;
>> }
>>
>> Now i think a shallow copy is done and if i destry only on the object
>> there would be a dangling pointer.
>> How do i overcome this problem as C structures don't support
>> functions?
>>

> Do it like this
>
> struct Sample *sample(int N)
> {
> struct Sample *answer;
> answer = malloc(sizeof(struct Sample));
> if(!answer)
> goto error_exit;
> answer.p = malloc(N);
> if(!answer.p)
> goto error_exit;
> return answer;
> error_exit:
> killSample(answer);
> return 0;
> }
>
> void killSample(struct Sample *s)
> {
> if(s)
> {
> free(s.p);
> free(s);
> }
> }
>
> struct Sample *Sample_clone(struct Sample *s)
> {
> struct Sample *answer;
>
> answer = sample(s.N):
> if(!answer)
> return 0;
> memcpy(answer.p, s.p, s.N);
> return answer;
> }

Depending on how you count them, you have about 8 errors and a rather
complex way of doing things. I would write:

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;
}

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.

--
Ben.

Richard

unread,
Jul 29, 2007, 9:38:24 AM7/29/07
to
pete <pfi...@mindspring.com> writes:

I think I have my dumb head on today Aunt Sally.

Really, how does the copy invoke undefined behaviour?

An undefined value is copied, but how does the invoke undefined
behaviour at the point of the copy?

regis

unread,
Jul 29, 2007, 10:09:52 AM7/29/07
to
Richard wrote:
> pete <pfi...@mindspring.com> writes:

>>>> Since a.i has an indeterminate value, the copy invokes undefined
>>>> behaviour.
>>> How?
>> Simply and obviously.
>
> I think I have my dumb head on today Aunt Sally.
>
> Really, how does the copy invoke undefined behaviour?
>
> An undefined value is copied, but how does the invoke undefined
> behaviour at the point of the copy?

suppose that moving an integer from memory to memory
requires to load it first into a register.
Now, suppose the undefined bit pattern of this integer
is a trap representation that triggers
an exception once the value is loaded in the register...

Harald van Dijk

unread,
Jul 29, 2007, 10:16:09 AM7/29/07
to

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.

santosh

unread,
Jul 29, 2007, 10:19:18 AM7/29/07
to
Harald van D?k wrote:

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

Richard

unread,
Jul 29, 2007, 10:16:18 AM7/29/07
to
regis <regis.ba...@free.fr> writes:

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.

santosh

unread,
Jul 29, 2007, 10:21:45 AM7/29/07
to
Richard wrote:

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

regis

unread,
Jul 29, 2007, 10:34:24 AM7/29/07
to

That was my line...

Richard

unread,
Jul 29, 2007, 10:23:50 AM7/29/07
to

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

regis

unread,
Jul 29, 2007, 10:42:40 AM7/29/07
to

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

unread,
Jul 29, 2007, 10:52:42 AM7/29/07
to

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

unread,
Jul 29, 2007, 11:15:07 AM7/29/07
to
santosh <santo...@gmail.com> writes:

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

Richard

unread,
Jul 29, 2007, 11:18:11 AM7/29/07
to
regis <regis.ba...@free.fr> writes:

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

unread,
Jul 29, 2007, 11:56:57 AM7/29/07
to
In article <6lwswjs...@gmail.com>, Richard <rgr...@gmail.com> wrote:
>regis <regis.ba...@free.fr> writes:
...

>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

unread,
Jul 29, 2007, 12:30:13 PM7/29/07
to

"Richard" <rgr...@gmail.com> wrote in message
news:6lwswjs...@gmail.com...

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

unread,
Jul 29, 2007, 12:36:00 PM7/29/07
to
"Malcolm McLean" <regn...@btinternet.com> writes:

> "Richard" <rgr...@gmail.com> wrote in message
> news:6lwswjs...@gmail.com...
>>
>> 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.

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

unread,
Jul 29, 2007, 12:43:35 PM7/29/07
to
In article <ox7ioj6...@gmail.com>, Richard <rgr...@gmail.com> wrote:
>"Malcolm McLean" <regn...@btinternet.com> writes:
>
>> "Richard" <rgr...@gmail.com> wrote in message
>> news:6lwswjs...@gmail.com...
>>>
>>> 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.
>
>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

unread,
Jul 29, 2007, 12:47:00 PM7/29/07
to

"Ben Bacarisse" <ben.u...@bsb.me.uk> wrote in message
news:87y7gz1...@bsb.me.uk...

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

unread,
Jul 29, 2007, 2:03:33 PM7/29/07
to

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.

--
pete

Keith Thompson

unread,
Jul 29, 2007, 4:22:31 PM7/29/07
to
santosh <santo...@gmail.com> writes:
> AnticitizenOne wrote:
>> On 29 Lug, 14:39, Richard Heathfield <r...@see.sig.invalid> wrote:
>>> 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_Other_Keith) ks...@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"

Keith Thompson

unread,
Jul 29, 2007, 4:45:03 PM7/29/07
to
santosh <santo...@gmail.com> writes:
> sam...@yahoo.co.in wrote:
>>
>> I have the following Struct,
>>
>> Struct Sample
>
> The keyword is struct, not Struct. C is case sensitive.
>
>> {
>> int i;
>> char *p;
>> };
>>
>> int main()
>> {
>> Sample a;
>
> The declaration should be:
>
> struct Sample a;
>
>> a.p = malloc(10);
>> Sample b;
>
> As above.
>
>> b = a;
>> }
>>
>> Now i think a shallow copy is done
>
> 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

unread,
Jul 29, 2007, 5:09:24 PM7/29/07
to
pete <pfi...@mindspring.com> writes:
[...]

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

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

Ben Bacarisse

unread,
Jul 29, 2007, 8:29:49 PM7/29/07
to
"Malcolm McLean" <regn...@btinternet.com> writes:

> "Ben Bacarisse" <ben.u...@bsb.me.uk> wrote in message
> news:87y7gz1...@bsb.me.uk...
>>
>> 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.

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.

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

<snip>


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

Ben Bacarisse

unread,
Jul 30, 2007, 7:45:05 AM7/30/07
to
Ben Bacarisse <ben.u...@bsb.me.uk> writes:

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

--
Ben.

Malcolm McLean

unread,
Jul 30, 2007, 4:49:14 PM7/30/07
to

"Ben Bacarisse" <ben.u...@bsb.me.uk> wrote in message
news:87abte2...@bsb.me.uk...
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

unread,
Jul 31, 2007, 1:47:26 PM7/31/07
to
"Malcolm McLean" <regn...@btinternet.com> writes:

> "Ben Bacarisse" <ben.u...@bsb.me.uk> wrote in message
> news:87abte2...@bsb.me.uk...
>> "Malcolm McLean" <regn...@btinternet.com> writes:
>>

<snip>

<snip>


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

--
Ben.

Chris Dollin

unread,
Jul 31, 2007, 2:38:08 PM7/31/07
to
Ben Bacarisse wrote:

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

--
Squinting Hedgehog
"Our future looks secure, but it's all out of our hands"
- Magenta, /Man and Machine/

0 new messages