[SLOF] [PATCH v2 01/11] libnet: Get rid of unused huge_load and block_size parameters

[Thomas Huth]

On 22.05.2018 16:49, Greg Kurz wrote:
> On Fri, 18 May 2018 17:45:30 +0200
> Thomas Huth <thuth at redhat.com> wrote:
> 
>> The blocksize is hard-coded to 1428 bytes in obp-tftp.fs, so instead of
>> hardcoding this in the Forth code, we could also move this into tftp.c
>> directly instead. A similar condition exists with the huge-tftp-load
>> parameter. While this non-standard variable could still be changed in the
>> obp-tftp package, it does not make much sense to set it to zero since you
>> only lose the possibility to do huge TFTP loads with index wrap-around in
>> that case.
>>
>> Signed-off-by: Thomas Huth <thuth at redhat.com>
>> ---
>>  lib/libnet/libnet.code       |  4 +---
>>  lib/libnet/netapps.h         |  3 +--
>>  lib/libnet/netload.c         | 11 ++++-------
>>  lib/libnet/tftp.c            | 13 +++----------
>>  lib/libnet/tftp.h            |  2 +-
>>  slof/fs/packages/obp-tftp.fs |  3 ---
>>  6 files changed, 10 insertions(+), 26 deletions(-)
>>
>> diff --git a/lib/libnet/libnet.code b/lib/libnet/libnet.code
>> index 2746782..419419d 100644
>> --- a/lib/libnet/libnet.code
>> +++ b/lib/libnet/libnet.code
>> @@ -4,11 +4,9 @@
>>  PRIM(NET_X2d_LOAD)
>>  	int alen = TOS.n; POP;
>>  	char *arg = TOS.a; POP;
>> -	int blocksize = TOS.n; POP;
>> -	int hugeload = TOS.n; POP;
>>  	long maxlen = TOS.n; POP;
>>  	void *loadaddr = TOS.a;
>> -	TOS.n = netload(loadaddr, maxlen, hugeload, blocksize, arg, alen);
>> +	TOS.n = netload(loadaddr, maxlen, arg, alen);
>>  MIRP
>>  
>>  PRIM(NET_X2d_PING)
>> diff --git a/lib/libnet/netapps.h b/lib/libnet/netapps.h
>> index 0e637e1..6e00466 100644
>> --- a/lib/libnet/netapps.h
>> +++ b/lib/libnet/netapps.h
>> @@ -18,8 +18,7 @@
>>  
>>  struct filename_ip;
>>  
>> -extern int netload(char *buffer, int len, int huge_load, int block_size,
>> -                   char *args_fs, int alen);
>> +extern int netload(char *buffer, int len, char *args_fs, int alen);
>>  extern int ping(char *args_fs, int alen);
>>  extern int dhcp(char *ret_buffer, struct filename_ip *fn_ip,
>>  		unsigned int retries, int flags);
>> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c
>> index 5c37fe2..8dca654 100644
>> --- a/lib/libnet/netload.c
>> +++ b/lib/libnet/netload.c
>> @@ -405,13 +405,12 @@ static void seed_rng(uint8_t mac[])
>>  }
>>  
>>  static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len,
>> -		     unsigned int retries, int32_t mode,
>> -		     int32_t blksize, int ip_vers)
>> +		     unsigned int retries, int ip_vers)
>>  {
>>  	tftp_err_t tftp_err;
>>  	int rc;
>>  
>> -	rc = tftp(fnip, buffer, len, retries, &tftp_err, mode, blksize, ip_vers);
>> +	rc = tftp(fnip, buffer, len, retries, &tftp_err, ip_vers);
>>  
>>  	if (rc > 0) {
>>  		printf("  TFTP: Received %s (%d KBytes)\n", fnip->filename,
>> @@ -510,8 +509,7 @@ static void encode_response(char *pkt_buffer, size_t size, int ip_init)
>>  	}
>>  }
>>  
>> -int netload(char *buffer, int len, int huge_load, int block_size,
>> -	    char *args_fs, int alen)
>> +int netload(char *buffer, int len, char *args_fs, int alen)
>>  {
>>  	int rc;
>>  	filename_ip_t fn_ip;
>> @@ -755,8 +753,7 @@ int netload(char *buffer, int len, int huge_load, int block_size,
>>  
>>  	/* Do the TFTP load and print error message if necessary */
>>  	rc = tftp_load(&fn_ip, (unsigned char *)buffer, len,
>> -		       obp_tftp_args.tftp_retries, huge_load,
>> -		       block_size, ip_version);
>> +		       obp_tftp_args.tftp_retries, ip_version);
>>  
>>  	if (obp_tftp_args.ip_init == IP_INIT_DHCP)
>>  		dhcp_send_release(fn_ip.fd);
>> diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c
>> index 1656c27..5e7951f 100644
>> --- a/lib/libnet/tftp.c
>> +++ b/lib/libnet/tftp.c
>> @@ -497,19 +497,15 @@ void handle_tftp_dun(uint8_t err_code)
>>   * @param  _len          size of destination buffer
>>   * @param  _retries      max number of retries
>>   * @param  _tftp_err     contains info about TFTP-errors (e.g. lost packets)
>> - * @param  _mode         NON ZERO - multicast, ZERO - unicast
>> - * @param  _blocksize    blocksize for DATA-packets
>>   * @return               ZERO - error condition occurs
>>   *                       NON ZERO - size of received file
>>   */
>>  int tftp(filename_ip_t * _fn_ip, unsigned char *_buffer, int _len,
>> -	 unsigned int _retries, tftp_err_t * _tftp_err,
>> -	 int32_t _mode, int32_t _blocksize, int _ip_version)
>> +	 unsigned int _retries, tftp_err_t * _tftp_err, int _ip_version)
>>  {
>>  	retries     = _retries;
>>  	fn_ip       = _fn_ip;
>>  	len         = _len;
>> -	huge_load   = _mode;
>>  	ip_version  = _ip_version;
>>  	tftp_errno  = 0;
>>  	tftp_err    = _tftp_err;
>> @@ -523,17 +519,14 @@ int tftp(filename_ip_t * _fn_ip, unsigned char *_buffer, int _len,
>>  	port_number = -1;
>>  	progress_first = -1;
>>  	progress_last_bytes = 0;
>> +	huge_load   = 1;
>>  
> 
> This static variable seems to have only two users, both in handle_tftp():
> 
>                 else if( block == 0xffff && huge_load != 0
>                      &&  (tftp->th_data == 0 || tftp->th_data == 1) ) {
>                         block = tftp->th_data;
>                 }
> 
> and
> 
>                 if (block >= 0xffff && huge_load == 0) {
>                         tftp_errno = -9;
>                         goto error;
>                 }
> 
> which could be simplified as well, but this can be done later I guess.

Yes, I thought about that, too, but I then decided that I'd rather leave
that code there ... if somebody ever wonders whether SLOF supports TFTP
loading with index roll-over (i.e. huge TFTP loads), it's more obvious
to spot this in the sources if we keep that "huge_load" variable in there.

> Reviewed-by: Greg Kurz <groug at kaod.org>

Thanks!

  Thomas