[flashrom] [SPAM?] Re: [PATCH 3/4] reformat -L output and add printing of chip voltage ranges to print.c

[Stefan Tauner]

On Sat, 25 Jun 2011 20:30:18 +0200
Stefan Tauner <stefan.tauner at student.tuwien.ac.at> wrote:

> On Sat, 25 Jun 2011 19:27:01 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
> 
> > Am 18.06.2011 22:53 schrieb Stefan Tauner:
> > >  	char *tmp_bus_str;
> > >  
> > >  	for (f = flashchips; f->name != NULL; f++) {
> > > @@ -87,12 +76,22 @@ static void print_supported_chips(void)
> > >  		if (!strncmp(f->name, "unknown", 7))
> > >  			continue;
> > >  		chipcount++;
> > > -		maxvendorlen = max(maxvendorlen, strlen(f->vendor));
> > > +		tmp_ven_str = (char *)f->vendor;
> > > +		do {
> > > +			tmp_ven_len = strcspn(tmp_ven_str, delim);
> > > +			maxvendorlen = max(maxvendorlen, tmp_ven_len);
> > > +			tmp_ven_str += tmp_ven_len;
> > >   
> > 
> > You need to add +1 to the expression above. Explanation follows.
> > 
> > 
> > > +		} while (tmp_ven_len != 0);
> > >   
> > 
> > Are you sure this loop does what it should?
> > Consider maxvendorlen=0, vendor="a/bc". The first iteration will have
> > tmp_ven_len=1, maxvendorlen=1, then tmp_ven_str="/bc". The next
> > iteration will have tmp_ven_len=0, maxvendorlen=1, tmp_ven_str="/bc" and
> > then the loop will stop with an incorrect value of maxvendorlen.
> 
> fixed and annotated with:
> /* skip to the char following the first '/' */
> thanks!
> 

actually it is not as easy that.
with your change added there is another "minor" problem :)
in the last token (or if there is only one) tmp_ven_str will point at
the memory address after '\0' resulting in strlen searching where we
surely don't want it to in the next iteration.

so basically the problem is that we can't distinguish between "token
found" and "end of string found".

my current solution is to check for that explicitly. 
checking for \0 and break;-ing is also possible. we could use while(1)
then if i see that right....

		do {
			tmp_ven_len = strcspn(tmp_ven_str, delim);
			maxvendorlen = max(maxvendorlen, tmp_ven_len);
			/* skip to the char following the first '/' */
			tmp_ven_str += tmp_ven_len;
			if (tmp_ven_str[0] == delim[0])
				tmp_ven_str++;
		} while (tmp_ven_len != 0);

the var[0] could be changed to *var, but i think it is clearer so.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner