gEDA-dev: Broken slotting
Peter Clifton
pcjc2 at cam.ac.uk
Tue Sep 11 06:12:09 EDT 2007
On Tue, 2007-09-11 at 11:04 +0200, Gabriel Paubert wrote:
> > /* Now put new pinseq attrib onto pin. */
> > new_pinseq = g_malloc(sizeof(char)*((numpins-1)*slot)+pin_counter);
>
> Indeed, it looks bogus:
> a) sizeof(char)==1 by definition (harmless)
> b) malloc has at least 4 byte granularity in practice
> c) 12 bytes will always be enough to keep the ASCII representation of
> an int in base 10. Of course I'm assuming that pinseq will not
> exceed about 2 billion, but that's already about 6 orders of
> magnitude more than the largest packages I know.
>
> > sprintf(new_pinseq, "%d", numpins*(slot-1)+pin_counter);
> > /* Add 1 for EOL char */
>
> I don't know whether we want to support negative pinseq but
> a safe way would be:
>
> char tmp[12];
> sprinf(tmp, "%d", numpins*(slot-1)+pin_counter);
> new_pinseq = g_strdup(tmp);
>
> The performance is probably not that worse. Double the
> size of tmp if you want to be 64-bit safe for multibillion
> (well, american billion) pin packages :-)
Thanks for the suggestions. Whilst multi-billion pin packages are a tad
unlikely, it feels wrong to hard-code it in a way which could prevent
them ;)
I was thinking I'd replace it with:
... = g_strdup_printf( "pinseq=%d", numpins*(slot-1)+pin_counter );
The pain (assuming its the correct fix) to accept pinseq=4 as a hit for
looking up "this slot's pin number 2" on a two pin component.
Code currently says "find me a pin on this component which matches
pinseq=2". I need to turn that around... saying "iterate over all pins
on this component, tell me what their pinseq is" - then test if the
pinseq is a wraps around to what we want.
Not a big problem, I just wanted to confirm other developers thought
this was the right fix (for now at least).
Regards,
Peter C.
More information about the geda-dev
mailing list