On Dec 19, 2007 3:17 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > +        cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
> > +            display "TFTP protocol allows negotiation of bigger packets.
> > +            Requires server which supports RFC 2348 blksize negotiation."
> > +            flavor  data
> > +            default_value 512
> > +            legal_values 512 to 65464
> > +            description   "
> > +             tftp blksize egotiation support. >512 byte block sizes improves
> > +             tftp GET performance"
> > +        }
>
> The display should be kept to one line maximum. Put all the rest on
> the description lines. You should also say that the default value of
> 512 causes this to be disabled. It might even be better to change the
> flavor of this to booldata, so it can be enabled/disabled and the
> value set. It then makes your code cleaner.

I fought w/booldata for a bit, but couldn't make sense of it.

> > +     // try without negotiating packet size. The serves that do
> > +     // not support options negotiation would normally just ignore
> > +     // the options and thus this code path will probably never be
> > +     // executed
>
> I took a very quick look at RFC 1350. I don't see it defining what to
> do when the RRQ has extra parameters. It does not say they should be
> ignored. So sending back an error is acceptable. That means i don't
> like this comment.

Comment fixed.

I tried w/two different servers. The one without options negotiation
just ignored
the options, so I haven't exercised the error code path.


>      Andrew
>
>

Better?



-- 
Øyvind Harboe
http://www.zylin.com - eCos ARM & FPGA  developer kit