Re: gEDA-user: Removing My* memory alllocation functions

[Peter Clifton <pcjc2@xxxxxxxxx>]

On Wed, 2010-12-08 at 14:26 +1100, Stephen Ecob wrote:
> Patch0002: we're getting close. I see only one remaining issue:
> 
> create.c:900: you address this both in patch 0001 and patch 0002
> In patch 0001 you preserve the behaviour of the existing code

It was still a save MyStrdup -> strdup change, so is valid there.

> In patch 0002 you unconditionally call strdup()

Was basically what you suggested I think, regarding TextString != NULL. 
I had presumed it was similar to the CreateNewText() case, but now you
point it out below, I see now why that might not have been correct.

> I've looked at the code further, and I think that the approach you
> take in patch 0001 is correct.
> Each element has three names: Description, NameOnPCB and Value.  It's
> entirely possible that at least one of them will be NULL, so an
> unconditional call would be dangerous.

> Please disregard my comment of 42 minutes ago "create.c:900 Same issue
> as create.c:593"  It's not the same issue.
> My $0.02 re create.c:593 still stands, but perhaps a better solution
> would be to put an assert(TextString != NULL); at the start of
> CreateNewText()

I'd be ok with that as a separate patch adding the assert. Beware that
asserts typically aren't switched on in a normal build of PCB though.

  TextTypePtr text = GetTextMemory (Layer);
  if (!text)
    return (text);

Love to hate that line.. What it means:

  if (text == NULL)
    return NULL;

It would be tempting to make that

  TextType *text;

  if (TextString == NULL)
    return NULL;

  text = GetTextMemory (Layer);
  if (text == NULL)
    return NULL;

  ....

But it does have the downside of making things fail silently. Perhaps
the assert is the best way to go.. it adds a note to the programmer of
what we expect, and the segfault in strdup will still catch any errors
if asserts are disabled.


> 
> _______________________________________________
> geda-user mailing list
> geda-user@xxxxxxxxxxxxxx
> http://www.seul.org/cgi-bin/mailman/listinfo/geda-user

-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)
Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)



_______________________________________________
geda-user mailing list
geda-user@xxxxxxxxxxxxxx
http://www.seul.org/cgi-bin/mailman/listinfo/geda-user