On Thu, Dec 9, 2010 at 12:10 AM, Peter Clifton <[email protected]> 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 [email protected] http://www.seul.org/cgi-bin/mailman/listinfo/geda-user
