Re: [avrdude-dev] Fix warnings patch

[E. Weddington]

On 28 Aug 2003 at 14:18, Theodore A. Roth wrote:

> 
> 
> On Thu, 28 Aug 2003, E. Weddington wrote:
> 
> > On 28 Aug 2003 at 13:53, Theodore A. Roth wrote:
> >
> > >
> > >
> > > On Thu, 28 Aug 2003, E. Weddington wrote:
> > >
> > > > Hello all,
> > > >
> > > > Attached is a patch that should fix the remaining warnings in building
> > > > the code. It should also not introduce any changes to functionality.
> > > >
> > > > I thought I'd post this in case anyone wants to give it a look over or
> > > > a good run through before I commit. Thanks.
> > >
> > > I don't think casting size and vsize to int is the right thing to do.
> > > Why not just change %d to %ld in the format specifier?
> > >
> > > Ted
> >
> > Both size and vsize are assigned their values from rc which is defined as
> > an int in that function, do_op().
> >
> > Perhaps the better thing to do would have been to change the type of size
> > and vsize from long to int, but I didn't want to second guess what the
> > intended use of these variables are for. Typecasting seemed safer.
> >
> > Hold on.... Am I getting bit by machine sizes again? I'm assuming that int
> > = 2 bytes. Is int = 4 bytes on Linux (etc.)?
> >
> > I've no problem changing the format spec to %ld if that's the better thing
> > to do.
> 
> Unless you are absolutely sure about what you are doing, casting is
> not usaully the right thing to do since you loose the ability to let
> the compiler do type checking. Probably not a big deal in this case
> though, but I've been bit by this way to many times.
> 

In the current code yes, because the assignment immediately preceding the 
fprintfs in question, here:

    size = rc;

    /*
     * write the buffer contents to the selected memory type
     */
    fprintf(stderr, "%s: writing %s (%d bytes):\n", 
            progname, upd->memtype, (int)size);


and here:

    vsize = rc;

    fprintf(stderr, "%s: %d bytes of %s written\n", progname, 
            (int)vsize, upd->memtype);


size and vsize get their value from rc which typed as an int.

Note, that this is why I rather would have wanted other people more 
familiar with the code (Brian?) to fix the warnings, because no matter what 
I do I would be second guessing the original programmers intentions.
1. Is rc supposed to be of type int? or long?
2. Is vsize and size supposed to be of type long? or type int?
3. Is the fprintf spec supposed to %d? or %ld?

This is why it's important to fix these warnings as soon as possible; it's 
difficult to remember / figure out later.

Given the code structure as it stands, typecasting is at the very least 
safe, even though I have no idea what the original intent was.

Eric