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

[Stephen Ecob <silicon.on.inspiration@xxxxxxxxx>]

On Thu, Dec 9, 2010 at 12:10 AM, Peter Clifton <pcjc2@xxxxxxxxx> wrote:
> On Wed, 2010-12-08 at 13:40 +1100, Stephen Ecob wrote:
>> Regarding 0002-Not-so-sure-about-these-MyStrdup-calls.patch:
>>
>> buffer.c:984: I can't tell, and suggest playing it safe with STRDUP()
>
>    if (line)
>      line->Number = strdup (pad->Number);
>
> I lumped this one in because we also used strdup for pad->Number when
> creating the pad.

Yes, it will be fine. A pad must always have a valid number.

>> create.c:593: My $0.02: CreateNewText() called with a NULL pointer
>> should be stopped in its tracks with a segfault rather than
>> propagating the error.  If you're unsure, though, just hit it with
>> STRDUP() - it preserves the current functionality.
>
> Tempting to make it a separate patch at the very least, deliberately
> stating in the commit that we are changing behaviour.

Yes, a separate patch would be better.

>> create.c:788 This is definitely *unsafe*, I have observed calls with
>> NULL pointers at run time.
>
> Your line-numbers don't agree with mine,

My line numbers are slightly higher than yours.  I suspect this is
because mine are pre patch 0001 whereas yours are post.  In many
places patch 0001 removes a line as a result of removing the second
parameter to MyStrdup().  For example:

-      NAMEONPCB_NAME (Element) = MyStrdup (line->Number,
-					   "ConvertBufferToElement");
+      NAMEONPCB_NAME (Element) = strdup (line->Number);

So your line numbers will be 1 less than mine for each such entry in patch 0001.

> but if this is pin->Name, I didn't change that one.. only pin->Number.

Oh yes, my mistake - I forgot that there are two calls on adjacent
lines.  strdup (Number) should be fine as a pin must always have a
number.

>> create.c:875 This is definitely *unsafe*, I have observed calls with
>> NULL pointers at run time.
>
> Same as above, I left pad->Name as MyStrdup, -> STRDUP in the mechanical
> patch.

Yes, same as above.

> TBH, there are so few cases left, we might even just expand the STRDUP
> macro in the places where it is required.

Sure, sounds good.


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