[U-Boot] [PATCH v3 3/7] dfu: DFU backend implementation

[Marek Vasut]

Dear Mike Frysinger,

> On Thursday 02 August 2012 09:55:24 Lukasz Majewski wrote:
> > Dear Mike Frysinger,
> > 
> > > On Tuesday 31 July 2012 02:36:59 Lukasz Majewski wrote:
> > > > +{
> > > > +	int i = 0;
> > > > +
> > > > +	for (; *s; s++)
> > > > +		if (*s == ';')
> > > > +			i++;
> > > > +
> > > > +	return ++i;
> > > > +}
> > 
> > In this function I count how many times the ';' separator appears.
> 
> well, to be more specific, i think it's counting the number of elements if
> you were to split on ';'.  so if there was one ';', this would return 2. 
> but you're correct of course that my suggested replacement is not
> equivalent.
> 
> > > > +int dfu_config_entities(char *env, char *interface, int num)
> > > > +{
> > > > +	struct dfu_entity *dfu;
> > > > +	int i, ret;
> > > > +	char *s;
> > > > +
> > > > +	dfu_alt_num = dfu_find_alt_num(env);
> > > > +	debug("%s: dfu_alt_num=%d\n", __func__, dfu_alt_num);
> > > > +
> > > > +	for (i = 0; i < dfu_alt_num; i++) {
> > > > +		dfu = calloc(sizeof(struct dfu_entity), 1);
> > > 
> > > seems like you can do this in a single call outside of the for loop:
> > > 	dfu = calloc(sizeof(*dfu), dfu_alt_num);
> > > 	if (!dfu)
> > > 	
> > > 		return -1;
> > > 	
> > > 	for (i = 0; i < dfu_alt_num; i++) {
> > > 	
> > > 		s = strsep(&env, ";");
> > > 		ret = dfu_fill_entity(&dfu[i], s, i, interface, num);
> > > 		if (ret)
> > > 		
> > > 			return -1;
> > > 		
> > > 		list_add_tail(&dfu[i].list, &dfu_list);
> > > 	
> > > 	}
> > 
> > I'd prefer to leave it as it is (since IMHO is more readable) with the
> 
> > addition of following code:
> it might be more slightly more readable, but doing a single function call
> results in better runtime performance

Doesn't the compiler optimize it as it sees fit?

Best regards,
Marek Vasut