[PATCH] Fix 2 possible SEGVs in tftp client

[Bernhard Fischer]

On Tue, Jun 13, 2006 at 02:07:43PM -0400, Rob Landley wrote:
>On Monday 24 April 2006 4:26 pm, Jason Schoon wrote:
>> Here is an updated patch.  First, I realized I had my file cleanup code in
>> the wrong place.  It should be called unconditionally, not in
>> FEATURE_CLEANUP.  I then went through and cleaned up some more of the code
>> a bit, removing useless comments and magic numbers as best I could.  I did
>> not yet try to shrink tftp() or switch to the ENABLE_ macros.
>
>Catching up on the backlog:
>
>+static const char MODE_OCTET[] = "octet";
>+static unsigned char MODE_OCTET_LEN = sizeof(MODE_OCTET);
>
>Statics (even const statics) actually wind up in the resulting binary.  

No.
It doesn't make any difference whatsoever if you
#define MODE_OCTET "octet"
or
static const char * const MODE_OCTET = "octet";
that i finally applied.

>#defines and enums do not (nor would just using sizeof() when you actually 
>need it.)
>
>In theory we can tell gcc "-fno-keep-static-consts" and 
>"-fmerge-all-constants" in order to avoid being _stupid_ about this, but I 
>just tried adding them and the resulting binary was identical...
>
>+/* RFC2348 says between 8 and 65464 */
>+#define TFTP_OCTECTS_MIN 8
>+#define TFTP_OCTECTS_MAX 65464
>...
>-           (blocksize < 8) || (blocksize > 65464)) {
>-               bb_error_msg("bad blocksize");
>-               return 0;
>+               (blocksize < TFTP_OCTECTS_MIN) || (blocksize > 
>TFTP_OCTECTS_MAX)) {
>+               bb_error_msg("bad blocksize");
>+               return 0;
>
>Please keep the constants inline but put the comment about how they're from 

No idea what you mean with 'inline' there.
We don't want magic numbers. Just look at an arbitrary driver written by
a vendor. It's unreadable because they tend to use magic numbers, 'cause
they are soooo l33t.

out-of-line would have been
int get_RFC2348_max_octets_value_as_an_int(void)
{ return 0xffb8;/* 0177670; see RFC2348 as shipped with your netware
diskettes */}

#define TFTP_OCTECTS_MAX 65464
*is* inline.

>RFC2348 where they're used.  (What is it supposed to buy you, moving a 
>constant that should never change out of line?)
>
>+#define tftp_cmd_get ENABLE_FEATURE_TFTP_GET
>
>Is there a down side to just replacing the uses of it with the correct macro?

I was lazy and wanted to keep the diff small. I'll remember to to expand
them next time, promised.
>
>Ah, nevermind.  I see Bernhard already applied it...
>
>Rob
>-- 
>Never bet against the cheap plastic solution.
>