[SLOF] [PATCH 3/9] libnet: Pass tftp_retries and ip_version via struct filename_ip

[Thomas Huth]

On 18.05.2018 12:09, Greg Kurz wrote:
> On Thu, 17 May 2018 19:40:50 +0200
> Thomas Huth <thuth at redhat.com> wrote:
> 
>> When we will support loading of pxelinux.cfg files later, we have to call
>> the tftp load function multiple times from different places. To avoid that
>> we've also got to pass around the tftp_retries and ip_version information
>> via function parameters to all spots, let's rather put them into struct
>> filename_ip since we've got this struct filename_ip info available every-
>> where already.
>>
>> Signed-off-by: Thomas Huth <thuth at redhat.com>
>> ---
>>  lib/libnet/netload.c | 11 ++++++-----
>>  lib/libnet/tftp.c    |  9 ++++-----
>>  lib/libnet/tftp.h    | 10 ++++++----
>>  3 files changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c
>> index 6b9bc57..add447c 100644
>> --- a/lib/libnet/netload.c
>> +++ b/lib/libnet/netload.c
>> @@ -404,13 +404,12 @@ static void seed_rng(uint8_t mac[])
>>  	srand(seed);
>>  }
>>  
>> -static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len,
>> -		     unsigned int retries, int ip_vers)
>> +static int tftp_load(filename_ip_t *fnip, void *buffer, int len)
>>  {
>>  	tftp_err_t tftp_err;
>>  	int rc;
>>  
>> -	rc = tftp(fnip, buffer, len, retries, &tftp_err, 1, 1428, ip_vers);
>> +	rc = tftp(fnip, buffer, len, &tftp_err, 1, 1428);
>>  
>>  	if (rc > 0) {
>>  		printf("  TFTP: Received %s (%d KBytes)\n", fnip->filename,
>> @@ -737,6 +736,9 @@ int netload(char *buffer, int len, char *args_fs, int alen)
>>  		fn_ip.filename[sizeof(fn_ip.filename)-1] = 0;
>>  	}
>>  
>> +	fn_ip.ip_version = ip_version;
>> +	fn_ip.tftp_retries = obp_tftp_args.tftp_retries;
>> +
>>  	if (ip_version == 4) {
>>  		printf("  Requesting file \"%s\" via TFTP from %d.%d.%d.%d\n",
>>  			fn_ip.filename,
>> @@ -752,8 +754,7 @@ int netload(char *buffer, int len, char *args_fs, int alen)
>>  	}
>>  
>>  	/* Do the TFTP load and print error message if necessary */
>> -	rc = tftp_load(&fn_ip, (unsigned char *)buffer, len,
>> -		       obp_tftp_args.tftp_retries, ip_version);
>> +	rc = tftp_load(&fn_ip, buffer, len);
>>  
>>  	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..d760f03 100644
>> --- a/lib/libnet/tftp.c
>> +++ b/lib/libnet/tftp.c
>> @@ -497,20 +497,19 @@ 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  _mode         NON ZERO - huge load, ZERO - no roll-over
>>   * @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)
>> +	 tftp_err_t * _tftp_err, int32_t _mode, int32_t _blocksize)
>>  {
>> -	retries     = _retries;
>> +	retries     = _fn_ip->tftp_retries;
>>  	fn_ip       = _fn_ip;
>>  	len         = _len;
>>  	huge_load   = _mode;
>> -	ip_version  = _ip_version;
>> +	ip_version  = _fn_ip->ip_version;
>>  	tftp_errno  = 0;
>>  	tftp_err    = _tftp_err;
>>  	tftp_err->bad_tftp_packets = 0;
>> diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h
>> index e32e473..6f68d15 100644
>> --- a/lib/libnet/tftp.h
>> +++ b/lib/libnet/tftp.h
>> @@ -29,8 +29,10 @@ struct filename_ip {
>>  	ip6_addr_t server_ip6;
>>  	ip6_addr_t dns_ip6;
>>  	char filename[256];
>> -	int    fd;
>> -} __attribute__ ((packed));
>> +	int fd;
>> +	int ip_version;
>> +	int tftp_retries;
>> +};
> 
> Why dropping the packed attribute ?

It does not seem to be necessary - this is just an internal structure
which is never given to or read from the outside.
I originally wanted to mention this in the patch description, but
apparently I forgot to do so :-(

> With this fixed or explained:
> 
> Greg Kurz <groug at kaod.org>

Reviewed-by? Acked-by?

 Thanks,
  Thomas