Re: [avrdude-dev] [RFC] stk500: skip empty pages

[E . Weddington]

> 
> 
> On Tue, 16 Sep 2003, E.Weddington wrote:
> 
> > > On Tue, Sep 16, 2003 at 05:38:10PM +0000, E.Weddington
> > wrote:
> > >
> > > > See attached.
> > >
> > > Looks good!
> > >
> >
> > Ok, this time, the redundant strcmps are worked around. 
See
> > attached. Let me know if ok to commit.
> 
> Why introduce the flash variable? Can't you just use
> 
>   if (memtype == 'F')
> 
> to get the same result?
> 
> Instead of this:
> 
> +    /* Only skip on empty page if programming flash. */
> +    if (flash) {
> +      if (stk500_is_page_empty(addr, page_size, m->buf)) 
{
> +          continue;
> +      }
> 
> I'd use this:
> 
> +    /* Only skip on empty page if programming flash. */
> +    if ((memtype == 'F') && stk500_is_page_empty(addr, 
page_size, m->buf)) {
> +        continue;
> +    }
> 

I didn't want to use memtype because it is specifically 
used later in part of the protocol itself:

    stk500_loadaddr(pgm, addr/a_div);
    buf[0] = Cmnd_STK_READ_PAGE;
    buf[1] = (page_size >> 8) & 0xff;
    buf[2] = page_size & 0xff;
    buf[3] = memtype;
    buf[4] = Sync_CRC_EOP;
    stk500_send(pgm, buf, 5);

and I'd rather have an independent flag that had nothing to 
do with the protocol in case, e.g. the protocol changed in 
a later revision.

I used two nesting ifs because I wasn't sure if with all 
the various compiler flags, compiler versions, etc., if 
short-circuit conditionals would be available and were 
used. I generally prefer to use them if available, but I 
was being conservative.

No questions about my use of continue, instead of another 
nesting if level? ;-)

Eric