[U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive

[Luca Ceresoli]

Detlev Zundel wrote:

> Hi Luca,
>
>> Signed-off-by: Luca Ceresoli<luca.ceresoli at comelit.it>
>> Cc: Wolfgang Denk<wd at denx.de>
>> ---
>> Changes in v2: none.
> Only one comment below
>
>>   net/tftp.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   net/tftp.h |    6 +++++
>>   2 files changed, 74 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/tftp.c b/net/tftp.c
>> index e166a63..87eb0b8 100644
>> --- a/net/tftp.c
>> +++ b/net/tftp.c
>> @@ -2,6 +2,8 @@
>>    *	Copyright 1994, 1995, 2000 Neil Russell.
>>    *	(See License)
>>    *	Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
>> + *	Copyright 2011 Comelit Group SpA,
>> + *	               Luca Ceresoli<luca.ceresoli at comelit.it>
>>    */
>>
>>   #include<common.h>
>> @@ -74,6 +76,7 @@ static short	TftpNumchars;	/* The number of hashes we printed      */
>>   #define STATE_TOO_LARGE	3
>>   #define STATE_BAD_MAGIC	4
>>   #define STATE_OACK	5
>> +#define STATE_RECV_WRQ	6
>>
>>   #define TFTP_BLOCK_SIZE		512		    /* default TFTP block size	*/
>>   #define TFTP_SEQUENCE_SIZE	((ulong)(1<<16))    /* sequence number is 16 bit */
>> @@ -241,6 +244,10 @@ TftpSend (void)
>>   			TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0);
>>   		/*..falling..*/
>>   #endif
>> +
>> +#ifdef CONFIG_CMD_TFTPSRV
>> +	case STATE_RECV_WRQ:
>> +#endif
>>   	case STATE_DATA:
>>   		xp = pkt;
>>   		s = (ushort *)pkt;
>> @@ -293,7 +300,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
>>   #endif
>>   		return;
>>   	}
>> -	if (TftpState != STATE_SEND_RRQ&&  src != TftpRemotePort)
>> +	if (TftpState != STATE_SEND_RRQ&&
>> +#ifdef CONFIG_CMD_TFTPSRV
>> +	    TftpState != STATE_RECV_WRQ&&
>> +#endif
>> +	    src != TftpRemotePort)
>>   		return;
> Hm, I have to admit that I do not really like adding so many ifdefs into
> the tftp code and even into the state machine code.  Can you give me a
> number on how much larger u-boot gets on your platform with and without
> the server support?  If this is not too much, maybe we should include
> support always.  If it is too much, maybe we can at least keep the state
> machine without ifdefs - if I see it correctly, this will only add at
> maximum a few bytes and STATE_RECW_WRQ will simply never be entered,
> correct?
Here are my measurements for the dig297 board (ARMV7 OMAP3):

    text    data     bss     dec     hex
  357085    8684  214264  580033   8d9c1 TFTPSRV on
  356327    8660  214264  579251   8d6b3 TFTPSRV off, without #ifs
  356355    8660  214268  579283   8d6d3 TFTPSRV off, with #ifs (as in PATCH v2)

The TFTP server adds 730 bytes to the text, so I guess it's worth to keep it
optional.

To my big surprise, removing most bad-looking #ifs (all of those that save just
one line) I got a *smaller* U-Boot! I repeated the test three times to be extra
sure, the figures are correct: without the #ifs we save a few bytes.

Obvious enough, I'm going to remove the #ifs.

Luca