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