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

[Greg Kurz]

On Fri, 18 May 2018 13:30:39 +0200
Thomas Huth <thuth at redhat.com> wrote:

> 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 :-(
> 

Ok, no big deal I guess.

> > With this fixed or explained:
> > 
> > Greg Kurz <groug at kaod.org>  
> 
> Reviewed-by? Acked-by?
> 

Oops, Reviewed-by :)

>  Thanks,
>   Thomas