[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..b86ca86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..9d87e2c 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -55,7 +55,7 @@ static int	TftpState;
 
 #define DEFAULT_NAME_LEN	(8 + 4 + 1)
 static char default_filename[DEFAULT_NAME_LEN];
-static char *tftp_filename;
+static char tftp_filename[2048];
 
 #ifdef CFG_DIRECT_FLASH_TFTP
 extern flash_info_t flash_info[];
@@ -231,7 +231,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -453,30 +453,38 @@ TftpStart (void)
 	char *ep;             /* Environment pointer */
 #endif
 
+	TftpServerIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
 			(NetOurIP >>  8) & 0xFF,
 			(NetOurIP >> 16) & 0xFF,
 			(NetOurIP >> 24) & 0xFF	);
-		tftp_filename = default_filename;
+		strcpy (tftp_filename, default_filename);
 
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p=BootFile;
+		p = strchr (p, ':');
+		if (p != NULL) {
+			TftpServerIP = string_to_ip (BootFile);
+			++p;
+			strcpy (tftp_filename, p);
+		} else 
+			strcpy (tftp_filename, BootFile);
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t ServerNet 	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
-- 
1.5.3.7

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:43 Sat 12 Jan     , Jean-Christophe PLAGNIOL-VILLARD wrote:
> allow to use a different server as set in serverip
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
Hi Ben,

	Could you review it,
	It works on arm and sh4 for me and compile on mips, ppc and nios2.

	I'd like to apply in the merge window.

Best Regards,
J.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> allow to use a different server as set in serverip
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index 21682c0..b86ca86 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	tftpboot,	3,	1,	do_tftpb,
>  	"tftpboot- boot image via network using TFTP protocol\n",
> -	"[loadAddress] [bootfilename]\n"
> +	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
>  );
>  
>  int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> diff --git a/net/tftp.c b/net/tftp.c
> index 8b95bcf..9d87e2c 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -34,7 +34,7 @@
>  #define TFTP_ERROR	5
>  #define TFTP_OACK	6
>  
> -
> +static IPaddr_t TftpServerIP;
>  static int	TftpServerPort;		/* The UDP port at their end		*/
>  static int	TftpOurPort;		/* The UDP port at our end		*/
>  static int	TftpTimeoutCount;
> @@ -55,7 +55,7 @@ static int	TftpState;
>  
>  #define DEFAULT_NAME_LEN	(8 + 4 + 1)
>  static char default_filename[DEFAULT_NAME_LEN];
> -static char *tftp_filename;
> +static char tftp_filename[2048];
>   
A 2k filename??? How about something more reasonable like 80 or 100 
bytes. Use a #define for the size so you can use it later.
>  
>  #ifdef CFG_DIRECT_FLASH_TFTP
>  extern flash_info_t flash_info[];
> @@ -231,7 +231,7 @@ TftpSend (void)
>  		break;
>  	}
>  
> -	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
> +	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
>  }
>  
>  
> @@ -453,30 +453,38 @@ TftpStart (void)
>  	char *ep;             /* Environment pointer */
>  #endif
>  
> +	TftpServerIP = NetServerIP;
>  	if (BootFile[0] == '\0') {
>  		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
>  			NetOurIP & 0xFF,
>  			(NetOurIP >>  8) & 0xFF,
>  			(NetOurIP >> 16) & 0xFF,
>  			(NetOurIP >> 24) & 0xFF	);
> -		tftp_filename = default_filename;
> +		strcpy (tftp_filename, default_filename);
>  
>   
Use strncpy, please, with the buffer length defined above.
>  		printf ("*** Warning: no boot file name; using '%s'\n",
>  			tftp_filename);
>  	} else {
> -		tftp_filename = BootFile;
> +		char *p=BootFile;
> +		p = strchr (p, ':');
> +		if (p != NULL) {
> +			TftpServerIP = string_to_ip (BootFile);
> +			++p;
> +			strcpy (tftp_filename, p);
>   
Again, strncpy please
> +		} else 
> +			strcpy (tftp_filename, BootFile);
>  	}
>  
>  #if defined(CONFIG_NET_MULTI)
>  	printf ("Using %s device\n", eth_get_name());
>  #endif
> -	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
> +	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
>  	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
>  
>  	/* Check if we need to send across this subnet */
>  	if (NetOurGatewayIP && NetOurSubnetMask) {
>  	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
> -	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
> +	    IPaddr_t ServerNet 	= TftpServerIP & NetOurSubnetMask;
>  
>  	    if (OurNet != ServerNet) {
>  		puts ("; sending through gateway ");
>   
This now works for me. Fix the file name buffer and I'll pull it in pronto.

regards,
Ben

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

On 22:19 Tue 15 Jan     , Ben Warren wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>> allow to use a different server as set in serverip
>>
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>
>> diff --git a/common/cmd_net.c b/common/cmd_net.c
>> index 21682c0..b86ca86 100644
>> --- a/common/cmd_net.c
>> +++ b/common/cmd_net.c
>> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char 
>> *argv[])
>>  U_BOOT_CMD(
>>  	tftpboot,	3,	1,	do_tftpb,
>>  	"tftpboot- boot image via network using TFTP protocol\n",
>> -	"[loadAddress] [bootfilename]\n"
>> +	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
>>  );
>>   int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>> diff --git a/net/tftp.c b/net/tftp.c
>> index 8b95bcf..9d87e2c 100644
>> --- a/net/tftp.c
>> +++ b/net/tftp.c
>> @@ -34,7 +34,7 @@
>>  #define TFTP_ERROR	5
>>  #define TFTP_OACK	6
>>  -
>> +static IPaddr_t TftpServerIP;
>>  static int	TftpServerPort;		/* The UDP port at their end		*/
>>  static int	TftpOurPort;		/* The UDP port at our end		*/
>>  static int	TftpTimeoutCount;
>> @@ -55,7 +55,7 @@ static int	TftpState;
>>   #define DEFAULT_NAME_LEN	(8 + 4 + 1)
>>  static char default_filename[DEFAULT_NAME_LEN];
>> -static char *tftp_filename;
>> +static char tftp_filename[2048];
>>   
> A 2k filename??? How about something more reasonable like 80 or 100 bytes. 
> Use a #define for the size so you can use it later.
I've chosed to use the same filename size as nfs.
>>   #ifdef CFG_DIRECT_FLASH_TFTP
>>  extern flash_info_t flash_info[];
>> @@ -231,7 +231,7 @@ TftpSend (void)
>>  		break;
>>  	}
>>  -	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, 
>> TftpOurPort, len);
>> +	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, 
>> TftpOurPort, len);
>>  }
>>   @@ -453,30 +453,38 @@ TftpStart (void)
>>  	char *ep;             /* Environment pointer */
>>  #endif
>>  +	TftpServerIP = NetServerIP;
>>  	if (BootFile[0] == '\0') {
>>  		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
>>  			NetOurIP & 0xFF,
>>  			(NetOurIP >>  8) & 0xFF,
>>  			(NetOurIP >> 16) & 0xFF,
>>  			(NetOurIP >> 24) & 0xFF	);
>> -		tftp_filename = default_filename;
>> +		strcpy (tftp_filename, default_filename);
>>    
> Use strncpy, please, with the buffer length defined above.
idem as my las comment copy from nfs
>>  		printf ("*** Warning: no boot file name; using '%s'\n",
>>  			tftp_filename);
>>  	} else {
>> -		tftp_filename = BootFile;
>> +		char *p=BootFile;
>> +		p = strchr (p, ':');
>> +		if (p != NULL) {
>> +			TftpServerIP = string_to_ip (BootFile);
>> +			++p;
>> +			strcpy (tftp_filename, p);
>>   
> Again, strncpy please
idem as my las comment copy from nfs
>> +		} else +			strcpy (tftp_filename, BootFile);
>>  	}
>>   #if defined(CONFIG_NET_MULTI)
>>  	printf ("Using %s device\n", eth_get_name());
>>  #endif
>> -	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
>> +	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
>>  	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
>>   	/* Check if we need to send across this subnet */
>>  	if (NetOurGatewayIP && NetOurSubnetMask) {
>>  	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
>> -	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
>> +	    IPaddr_t ServerNet 	= TftpServerIP & NetOurSubnetMask;
>>   	    if (OurNet != ServerNet) {
>>  		puts ("; sending through gateway ");
>>   
> This now works for me. Fix the file name buffer and I'll pull it in pronto.
>
> regards,
> Ben
Best Regards,
J.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 22:19 Tue 15 Jan     , Ben Warren wrote:
>   
>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>     
>>> allow to use a different server as set in serverip
>>>
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>
>>> diff --git a/common/cmd_net.c b/common/cmd_net.c
>>> index 21682c0..b86ca86 100644
>>> --- a/common/cmd_net.c
>>> +++ b/common/cmd_net.c
>>> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char 
>>> *argv[])
>>>  U_BOOT_CMD(
>>>  	tftpboot,	3,	1,	do_tftpb,
>>>  	"tftpboot- boot image via network using TFTP protocol\n",
>>> -	"[loadAddress] [bootfilename]\n"
>>> +	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
>>>  );
>>>   int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>> diff --git a/net/tftp.c b/net/tftp.c
>>> index 8b95bcf..9d87e2c 100644
>>> --- a/net/tftp.c
>>> +++ b/net/tftp.c
>>> @@ -34,7 +34,7 @@
>>>  #define TFTP_ERROR	5
>>>  #define TFTP_OACK	6
>>>  -
>>> +static IPaddr_t TftpServerIP;
>>>  static int	TftpServerPort;		/* The UDP port at their end		*/
>>>  static int	TftpOurPort;		/* The UDP port at our end		*/
>>>  static int	TftpTimeoutCount;
>>> @@ -55,7 +55,7 @@ static int	TftpState;
>>>   #define DEFAULT_NAME_LEN	(8 + 4 + 1)
>>>  static char default_filename[DEFAULT_NAME_LEN];
>>> -static char *tftp_filename;
>>> +static char tftp_filename[2048];
>>>   
>>>       
>> A 2k filename??? How about something more reasonable like 80 or 100 bytes. 
>> Use a #define for the size so you can use it later.
>>     
> I've chosed to use the same filename size as nfs.
>   
Then NFS uses too big a buffer. Please make this one smaller, use a 
#define and strncpy.
<snip>

regards,
Ben

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:49 Wed 16 Jan     , Ben Warren wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 22:19 Tue 15 Jan     , Ben Warren wrote:
> >   
> >> Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>     
> >>> allow to use a different server as set in serverip
> >>>
> >>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >>>
> >>> diff --git a/common/cmd_net.c b/common/cmd_net.c
> >>> index 21682c0..b86ca86 100644
> >>> --- a/common/cmd_net.c
> >>> +++ b/common/cmd_net.c
> >>> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char 
> >>> *argv[])
> >>>  U_BOOT_CMD(
> >>>  	tftpboot,	3,	1,	do_tftpb,
> >>>  	"tftpboot- boot image via network using TFTP protocol\n",
> >>> -	"[loadAddress] [bootfilename]\n"
> >>> +	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
> >>>  );
> >>>   int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> >>> diff --git a/net/tftp.c b/net/tftp.c
> >>> index 8b95bcf..9d87e2c 100644
> >>> --- a/net/tftp.c
> >>> +++ b/net/tftp.c
> >>> @@ -34,7 +34,7 @@
> >>>  #define TFTP_ERROR	5
> >>>  #define TFTP_OACK	6
> >>>  -
> >>> +static IPaddr_t TftpServerIP;
> >>>  static int	TftpServerPort;		/* The UDP port at their end		*/
> >>>  static int	TftpOurPort;		/* The UDP port at our end		*/
> >>>  static int	TftpTimeoutCount;
> >>> @@ -55,7 +55,7 @@ static int	TftpState;
> >>>   #define DEFAULT_NAME_LEN	(8 + 4 + 1)
> >>>  static char default_filename[DEFAULT_NAME_LEN];
> >>> -static char *tftp_filename;
> >>> +static char tftp_filename[2048];
> >>>   
> >>>       
> >> A 2k filename??? How about something more reasonable like 80 or 100 bytes. 
> >> Use a #define for the size so you can use it later.
> >>     
> > I've chosed to use the same filename size as nfs.
> >   
> Then NFS uses too big a buffer. Please make this one smaller, use a 
> #define and strncpy.
> <snip>
OK about the buffer size.

But I diaagree about strncpy because you need to known the size of
the string and we know it only when we use the default_filename otherwise
we need to strlen and in this strcpy do every thing itself.

Best Regards,
J.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:49 Wed 16 Jan     , Ben Warren wrote:
>   
>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>     
>>> On 22:19 Tue 15 Jan     , Ben Warren wrote:
>>>   
>>>       
>>>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>     
>>>>         
>>>>> allow to use a different server as set in serverip
>>>>>
>>>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>>>
>>>>> diff --git a/common/cmd_net.c b/common/cmd_net.c
>>>>> index 21682c0..b86ca86 100644
>>>>> --- a/common/cmd_net.c
>>>>> +++ b/common/cmd_net.c
>>>>> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char 
>>>>> *argv[])
>>>>>  U_BOOT_CMD(
>>>>>  	tftpboot,	3,	1,	do_tftpb,
>>>>>  	"tftpboot- boot image via network using TFTP protocol\n",
>>>>> -	"[loadAddress] [bootfilename]\n"
>>>>> +	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
>>>>>  );
>>>>>   int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>>>> diff --git a/net/tftp.c b/net/tftp.c
>>>>> index 8b95bcf..9d87e2c 100644
>>>>> --- a/net/tftp.c
>>>>> +++ b/net/tftp.c
>>>>> @@ -34,7 +34,7 @@
>>>>>  #define TFTP_ERROR	5
>>>>>  #define TFTP_OACK	6
>>>>>  -
>>>>> +static IPaddr_t TftpServerIP;
>>>>>  static int	TftpServerPort;		/* The UDP port at their end		*/
>>>>>  static int	TftpOurPort;		/* The UDP port at our end		*/
>>>>>  static int	TftpTimeoutCount;
>>>>> @@ -55,7 +55,7 @@ static int	TftpState;
>>>>>   #define DEFAULT_NAME_LEN	(8 + 4 + 1)
>>>>>  static char default_filename[DEFAULT_NAME_LEN];
>>>>> -static char *tftp_filename;
>>>>> +static char tftp_filename[2048];
>>>>>   
>>>>>       
>>>>>           
>>>> A 2k filename??? How about something more reasonable like 80 or 100 bytes. 
>>>> Use a #define for the size so you can use it later.
>>>>     
>>>>         
>>> I've chosed to use the same filename size as nfs.
>>>   
>>>       
>> Then NFS uses too big a buffer. Please make this one smaller, use a 
>> #define and strncpy.
>> <snip>
>>     
> OK about the buffer size.
>
> But I diaagree about strncpy because you need to known the size of
> the string and we know it only when we use the default_filename otherwise
> we need to strlen and in this strcpy do every thing itself.
>
>   
strncpy copies AT MOST 'n' bytes.

#define MAX_FILE_NAME_LEN 80
static char tftp_filename[MAX_FILE_NAME_LEN];

strncpy(tftp_filename, str, MAX_FILE_NAME_LEN);

Standard buffer overrun protection.

regards,
Ben

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Wolfgang Denk]

In message <478E68C6.8070309@gmail.com> you wrote:
>
> > But I diaagree about strncpy because you need to known the size of
> > the string and we know it only when we use the default_filename otherwise
> > we need to strlen and in this strcpy do every thing itself.
> >
> >   
> strncpy copies AT MOST 'n' bytes.

...and doesn't necessarily NUL-terminate.

> #define MAX_FILE_NAME_LEN 80
> static char tftp_filename[MAX_FILE_NAME_LEN];
> 
> strncpy(tftp_filename, str, MAX_FILE_NAME_LEN);
> 
> Standard buffer overrun protection.

...only if you make sure that tftp_filename[] gets terminated
independent of the length; otherwise you fix the problem when writing
the srting but create one when reading it.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Every program has at least one bug and can be shortened by  at  least
one instruction - from which, by induction, one can deduce that every
program can be reduced to one instruction which doesn't work.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Wolfgang Denk wrote:
> In message <478E68C6.8070309@gmail.com> you wrote:
>   
>>> But I diaagree about strncpy because you need to known the size of
>>> the string and we know it only when we use the default_filename otherwise
>>> we need to strlen and in this strcpy do every thing itself.
>>>
>>>   
>>>       
>> strncpy copies AT MOST 'n' bytes.
>>     
>
> ...and doesn't necessarily NUL-terminate.
>
>   
>> #define MAX_FILE_NAME_LEN 80
>> static char tftp_filename[MAX_FILE_NAME_LEN];
>>
>> strncpy(tftp_filename, str, MAX_FILE_NAME_LEN);
>>
>> Standard buffer overrun protection.
>>     
>
> ...only if you make sure that tftp_filename[] gets terminated
> independent of the length; otherwise you fix the problem when writing
> the srting but create one when reading it.
>
> Best regards,
>
> Wolfgang Denk
>
>   
#define MAX_LEN 80
static char tftp_filename[MAX_LEN + 1];
memset(tftp_filename[MAX_LEN], 0, 1);

strncpy(tftp_filename, str, MAX_LEN);


Better?

regards,
Ben

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[McMullan, Jason]

This one gets my vote:


#define MAX_LEN 80
static char tftp_filename[MAX_LEN];

strncpy(tftp_filename, str, MAX_LEN);
tftp_filename[MAX_LEN-1] = 0;

May be truncated, but never overruns.

-  
Jason McMullan
Network Appliance, Inc.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080116/5d558cdf/attachment.pgp 

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Wolfgang Denk]

In message <1200517000.31842.70.camel@localhost> you wrote:
> 
> #define MAX_LEN 80
> static char tftp_filename[MAX_LEN];
> 
> strncpy(tftp_filename, str, MAX_LEN);
> tftp_filename[MAX_LEN-1] = 0;
> 
> May be truncated, but never overruns.

Acked-by: wd at denx.de :-)


Except that the length should be 128 to  match  the  boot  file  name
length that BOOTP / DHCP can pass as per RFC.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A committee is a group that keeps the minutes and loses hours.
                                                      -- Milton Berle

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

On 22:26 Wed 16 Jan     , Wolfgang Denk wrote:
> In message <1200517000.31842.70.camel@localhost> you wrote:
> > 
> > #define MAX_LEN 80
> > static char tftp_filename[MAX_LEN];
> > 
> > strncpy(tftp_filename, str, MAX_LEN);
> > tftp_filename[MAX_LEN-1] = 0;
> > 
> > May be truncated, but never overruns.
> 
> Acked-by: wd at denx.de :-)
> 
> 
> Except that the length should be 128 to  match  the  boot  file  name
> length that BOOTP / DHCP can pass as per RFC.

Personnaly, I'll prefer 1K length because if you use as I use a lost of
time a full path when downloading uImage or other so a path with a
length over than 512 arrive often.

And with nfs it's the same

Best Regards,
J.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Wolfgang Denk]

In message <20080116222754.GG31365@game.jcrosoft.org> you wrote:
>
> > Except that the length should be 128 to  match  the  boot  file  name
> > length that BOOTP / DHCP can pass as per RFC.
> 
> Personnaly, I'll prefer 1K length because if you use as I use a lost of
> time a full path when downloading uImage or other so a path with a
> length over than 512 arrive often.

So is there a newer RFC that changes this? IIRC the max length is
defined as 128 bytes.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There is, however, a strange, musty smell in the air that reminds  me
of something...hmm...yes...I've got it...there's a VMS nearby, or I'm
a Blit.          - Larry Wall in Configure from the perl distribution

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Wolfgang Denk wrote:
> In message <20080116222754.GG31365@game.jcrosoft.org> you wrote:
>   
>>> Except that the length should be 128 to  match  the  boot  file  name
>>> length that BOOTP / DHCP can pass as per RFC.
>>>       
>> Personnaly, I'll prefer 1K length because if you use as I use a lost of
>> time a full path when downloading uImage or other so a path with a
>> length over than 512 arrive often.
>>     
>
> So is there a newer RFC that changes this? IIRC the max length is
> defined as 128 bytes.
>
> Best regards,
>
> Wolfgang Denk
>
>   
My 2c is that we go with 128, and have a CONFIG_LONG_TFTP_FILE_NAME that 
allows 1k for people with ridiculously long paths.

regards,
Ben

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip
add CONFIG_LONG_TFTP_FILE_NAME to allow long file name default as 1024
can be reconfigured whit CONFIG_LONG_TFTP_FILE_NAME_MAX_LEN

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..b86ca86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..262ff79 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -55,7 +55,18 @@ static int	TftpState;
 
 #define DEFAULT_NAME_LEN	(8 + 4 + 1)
 static char default_filename[DEFAULT_NAME_LEN];
-static char *tftp_filename;
+
+#ifdef CONFIG_LONG_TFTP_FILE_NAME
+# ifndef CONFIG_LONG_TFTP_FILE_NAME_MAX_LEN
+#  define MAX_LEN 1024
+# else
+#  define MAX_LEN CONFIG_LONG_TFTP_FILE_NAME_MAX_LEN
+# endif
+#else
+#define MAX_LEN 128
+#endif
+
+static char tftp_filename[MAX_LEN];
 
 #ifdef CFG_DIRECT_FLASH_TFTP
 extern flash_info_t flash_info[];
@@ -231,7 +242,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -372,7 +383,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
 #ifdef CONFIG_MCAST_TFTP
 		/* if I am the MasterClient, actively calculate what my next
 		 * needed block is; else I'm passive; not ACKING
- 		 */
+		 */
 		if (Multicast) {
 			if (len < TftpBlkSize)  {
 				TftpEndingBlock = TftpBlock;
@@ -453,30 +464,43 @@ TftpStart (void)
 	char *ep;             /* Environment pointer */
 #endif
 
+	TftpServerIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
 			(NetOurIP >>  8) & 0xFF,
 			(NetOurIP >> 16) & 0xFF,
 			(NetOurIP >> 24) & 0xFF	);
-		tftp_filename = default_filename;
+
+		strncpy(tftp_filename, default_filename, MAX_LEN);
+		tftp_filename[MAX_LEN-1] = 0;
 
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p=BootFile;
+		p = strchr (p, ':');
+		if (p != NULL) {
+			TftpServerIP = string_to_ip (BootFile);
+			++p;
+			strncpy(tftp_filename, p, MAX_LEN);
+			tftp_filename[MAX_LEN-1] = 0;
+		} else {
+			strncpy(tftp_filename, BootFile, MAX_LEN);
+			tftp_filename[MAX_LEN-1] = 0;
+		}
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
-	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t OurNet	= NetOurIP    & NetOurSubnetMask;
+	    IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
@@ -522,7 +546,7 @@ TftpStart (void)
 	/* Revert TftpBlkSize to dflt */
 	TftpBlkSize = TFTP_BLOCK_SIZE;
 #ifdef CONFIG_MCAST_TFTP
-    	mcast_cleanup();
+	mcast_cleanup();
 #endif
 
 	TftpSend ();
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index b86ca86..dbf6b86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	nfs,	3,	1,	do_nfs,
 	"nfs\t- boot image via network using NFS protocol\n",
-	"[loadAddress] [host ip addr:bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 #endif
 
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Ben Warren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index b86ca86..dbf6b86 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	nfs,	3,	1,	do_nfs,
>  	"nfs\t- boot image via network using NFS protocol\n",
> -	"[loadAddress] [host ip addr:bootfilename]\n"
> +	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
>  );
>  #endif
>  
>   
This one's already in.

regards,
Ben

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Wolfgang Denk]

In message <1200607712-1381-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> allow to use a different server as set in serverip
> add CONFIG_LONG_TFTP_FILE_NAME to allow long file name default as 1024
> can be reconfigured whit CONFIG_LONG_TFTP_FILE_NAME_MAX_LEN
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/net/tftp.c b/net/tftp.c
> index 8b95bcf..262ff79 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
...
> @@ -55,7 +55,18 @@ static int	TftpState;
>  
>  #define DEFAULT_NAME_LEN	(8 + 4 + 1)
>  static char default_filename[DEFAULT_NAME_LEN];
> -static char *tftp_filename;
> +
> +#ifdef CONFIG_LONG_TFTP_FILE_NAME
> +# ifndef CONFIG_LONG_TFTP_FILE_NAME_MAX_LEN
> +#  define MAX_LEN 1024
> +# else
> +#  define MAX_LEN CONFIG_LONG_TFTP_FILE_NAME_MAX_LEN
> +# endif
> +#else
> +#define MAX_LEN 128
> +#endif
> +
> +static char tftp_filename[MAX_LEN];

We don't need *two* new defines. Just use one, say
CONFIG_TFTP_FILE_NAME_MAX_LEN

> @@ -372,7 +383,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
>  #ifdef CONFIG_MCAST_TFTP
>  		/* if I am the MasterClient, actively calculate what my next
>  		 * needed block is; else I'm passive; not ACKING
> - 		 */
> +		 */
>  		if (Multicast) {
>  			if (len < TftpBlkSize)  {
>  				TftpEndingBlock = TftpBlock;
> @@ -453,30 +464,43 @@ TftpStart (void)
>  	char *ep;             /* Environment pointer */
>  #endif
>  
> +	TftpServerIP = NetServerIP;
>  	if (BootFile[0] == '\0') {
>  		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
>  			NetOurIP & 0xFF,
>  			(NetOurIP >>  8) & 0xFF,
>  			(NetOurIP >> 16) & 0xFF,
>  			(NetOurIP >> 24) & 0xFF	);
> -		tftp_filename = default_filename;
> +
> +		strncpy(tftp_filename, default_filename, MAX_LEN);
> +		tftp_filename[MAX_LEN-1] = 0;
>  
>  		printf ("*** Warning: no boot file name; using '%s'\n",
>  			tftp_filename);
>  	} else {
> -		tftp_filename = BootFile;
> +		char *p=BootFile;
> +		p = strchr (p, ':');

Blank line after declarations. And make this:

		char *p = strchr (BootFile, ':');

> +		if (p != NULL) {

Please swap the "if" and "else" branches and use "if (p == NULL)"


> +			TftpServerIP = string_to_ip (BootFile);

Note that there is no terminating '0' at *p ! You might want to insert
one for robustness.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Vulcans worship peace above all.
	-- McCoy, "Return to Tomorrow", stardate 4768.3

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Andre Renaud]

Couldn't this be done as follows instead? This way there is no need to allocate any more 
memory on the stack, and no need for a new configuration option.

Andre

Signed-off-by: Andre Renaud <andre@bluewatesys.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..b86ca86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
     tftpboot,    3,    1,    do_tftpb,
     "tftpboot- boot image via network using TFTP protocol\n",
-    "[loadAddress] [bootfilename]\n"
+    "[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..262ff79 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR    5
 #define TFTP_OACK    6
 
-
+static IPaddr_t TftpServerIP;
 static int    TftpServerPort;        /* The UDP port at their end        */
 static int    TftpOurPort;        /* The UDP port at our end        */
 static int    TftpTimeoutCount;
@@ -231,7 +242,7 @@ TftpSend (void)
         break;
     }
 
-    NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+    NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -453,30 +464,43 @@ TftpStart (void)
     char *ep;             /* Environment pointer */
 #endif
 
+    TftpServerIP = NetServerIP;
     if (BootFile[0] == '\0') {
         sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
             NetOurIP & 0xFF,
             (NetOurIP >>  8) & 0xFF,
             (NetOurIP >> 16) & 0xFF,
             (NetOurIP >> 24) & 0xFF    );
         tftp_filename = default_filename;
 
         printf ("*** Warning: no boot file name; using '%s'\n",
             tftp_filename);
     } else {
+        char *p=BootFile;
+        p = strchr (p, ':');
+        if (p != NULL) {
+            TftpServerIP = string_to_ip (BootFile);
+            ++p;
+            tftp_filename = p;
+        } else
+            tftp_filename = BootFile;
     }
 
 #if defined(CONFIG_NET_MULTI)
     printf ("Using %s device\n", eth_get_name());
 #endif
-    puts ("TFTP from server ");    print_IPaddr (NetServerIP);
+    puts ("TFTP from server ");    print_IPaddr (TftpServerIP);
     puts ("; our IP address is ");    print_IPaddr (NetOurIP);
 
     /* Check if we need to send across this subnet */
     if (NetOurGatewayIP && NetOurSubnetMask) {
-        IPaddr_t OurNet     = NetOurIP    & NetOurSubnetMask;
-        IPaddr_t ServerNet     = NetServerIP & NetOurSubnetMask;
+        IPaddr_t OurNet    = NetOurIP    & NetOurSubnetMask;
+        IPaddr_t ServerNet    = TftpServerIP & NetOurSubnetMask;
 
         if (OurNet != ServerNet) {
         puts ("; sending through gateway ");

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Ben Warren]

Andre Renaud wrote:
> Couldn't this be done as follows instead? This way there is no need to allocate any more 
> memory on the stack, and no need for a new configuration option.
>
>   
This is nice, because it works in-place and doesn't need the second 
buffer. One concern, though (see below)
> Andre
>
> Signed-off-by: Andre Renaud <andre@bluewatesys.com>
>
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index 21682c0..b86ca86 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>      tftpboot,    3,    1,    do_tftpb,
>      "tftpboot- boot image via network using TFTP protocol\n",
> -    "[loadAddress] [bootfilename]\n"
> +    "[loadAddress] [[hostIPaddr:]bootfilename]\n"
>  );
>  
>  int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> diff --git a/net/tftp.c b/net/tftp.c
> index 8b95bcf..262ff79 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -34,7 +34,7 @@
>  #define TFTP_ERROR    5
>  #define TFTP_OACK    6
>  
> -
> +static IPaddr_t TftpServerIP;
>  static int    TftpServerPort;        /* The UDP port at their end        */
>  static int    TftpOurPort;        /* The UDP port at our end        */
>  static int    TftpTimeoutCount;
> @@ -231,7 +242,7 @@ TftpSend (void)
>          break;
>      }
>  
> -    NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
> +    NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
>  }
>  
>  
> @@ -453,30 +464,43 @@ TftpStart (void)
>      char *ep;             /* Environment pointer */
>  #endif
>  
> +    TftpServerIP = NetServerIP;
>      if (BootFile[0] == '\0') {
>          sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
>              NetOurIP & 0xFF,
>              (NetOurIP >>  8) & 0xFF,
>              (NetOurIP >> 16) & 0xFF,
>              (NetOurIP >> 24) & 0xFF    );
>          tftp_filename = default_filename;
>  
>          printf ("*** Warning: no boot file name; using '%s'\n",
>              tftp_filename);
>      } else {
> +        char *p=BootFile;
> +        p = strchr (p, ':');
> +        if (p != NULL) {
> +            TftpServerIP = string_to_ip (BootFile);
>   
I don't know if string_to_ip() works properly on a string of the format 
a.b.c.d:filename
> +            ++p;
> +            tftp_filename = p;
> +        } else
> +            tftp_filename = BootFile;
>      }
>  
>   
Alternatively:

	char *p = strchr(Bootfile, ':');
	if (p && *(p+1) != '\0') {
		tftp_filename = P + 1;
		*p = '\0';
		TftpServerIP = string_to_ip(Bootfile);
	} else
		tftp_filename = Bootfile;


*** Note: I haven't tried compiling this and it was written quickly, so 
may be garbage! Just ask Wolfgang...

regards,
Ben

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:42 Fri 18 Jan     , Andre Renaud wrote:
> Couldn't this be done as follows instead? This way there is no need to allocate any more 
> memory on the stack, and no need for a new configuration option.
> 
> Andre
> 
> Signed-off-by: Andre Renaud <andre@bluewatesys.com>
> 

Andren,
	At least add my Signed-off

Best Regards,
J.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Wolfgang Denk]

In message <478E6D5A.4070807@gmail.com> you wrote:
>
> #define MAX_LEN 80
> static char tftp_filename[MAX_LEN + 1];
> memset(tftp_filename[MAX_LEN], 0, 1);

warning: passing argument 1 of 'memset' makes pointer from integer without a cast

> strncpy(tftp_filename, str, MAX_LEN);
> 
> 
> Better?

No, definitely not.

First, the compiler will issue a warning; second, using  memcpy()  to
store  a  single character is serious overkill - why don't you simply
use "tftp_filename[MAX_LEN] = '\0';" ?;and third - the real bug - the
strncpy will happily overwrite the 0 you placed there before.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I can call spirits from the vasty deep."
"Why so can I, or so can any man; but will they come when you do call
for them?"          - Shakespeare, 1 King Henry IV, Act III, Scene I.

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Wolfgang Denk wrote:
> In message <478E6D5A.4070807@gmail.com> you wrote:
>   
>> #define MAX_LEN 80
>> static char tftp_filename[MAX_LEN + 1];
>> memset(tftp_filename[MAX_LEN], 0, 1);
>>     
>
> warning: passing argument 1 of 'memset' makes pointer from integer without a cast
>
>   
10-second response, brain not fully engaged.  Compiler not used (duh?)
>> strncpy(tftp_filename, str, MAX_LEN);
>>
>>
>> Better?
>>     
>
> No, definitely not.
>
> First, the compiler will issue a warning; second, using  memcpy()  to
> store  a  single character is serious overkill - why don't you simply
> use "tftp_filename[MAX_LEN] = '\0';" ?;and third - the real bug - the
> strncpy will happily overwrite the 0 you placed there before.
>
>   
strncpy won't touch the zero, since it's in the 81st byte and strncpy 
will only copy 80.  True about the warnings and memset overkill, though.

OK, let's move on...

Jean-Christophe - Jason McMullen has given you a nice prototype.  Please 
use it.

regards,
Ben

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip
add CONFIG_TFTP_FILE_NAME_MAX_LEN to configure the file name length
if not defined the max length will be at 128

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..b86ca86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..2096178 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -55,7 +55,14 @@ static int	TftpState;
 
 #define DEFAULT_NAME_LEN	(8 + 4 + 1)
 static char default_filename[DEFAULT_NAME_LEN];
-static char *tftp_filename;
+
+#ifndef CONFIG_TFTP_FILE_NAME_MAX_LEN
+#define MAX_LEN 128
+#else
+#define MAX_LEN CONFIG_TFTP_FILE_NAME_MAX_LEN
+#endif
+
+static char tftp_filename[MAX_LEN];
 
 #ifdef CFG_DIRECT_FLASH_TFTP
 extern flash_info_t flash_info[];
@@ -231,7 +238,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -372,7 +379,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
 #ifdef CONFIG_MCAST_TFTP
 		/* if I am the MasterClient, actively calculate what my next
 		 * needed block is; else I'm passive; not ACKING
- 		 */
+		 */
 		if (Multicast) {
 			if (len < TftpBlkSize)  {
 				TftpEndingBlock = TftpBlock;
@@ -453,30 +460,42 @@ TftpStart (void)
 	char *ep;             /* Environment pointer */
 #endif
 
+	TftpServerIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
 			(NetOurIP >>  8) & 0xFF,
 			(NetOurIP >> 16) & 0xFF,
 			(NetOurIP >> 24) & 0xFF	);
-		tftp_filename = default_filename;
+
+		strncpy(tftp_filename, default_filename, MAX_LEN);
+		tftp_filename[MAX_LEN-1] = 0;
 
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p = strchr (p, ':');
+		if (p == NULL) {
+			strncpy(tftp_filename, BootFile, MAX_LEN);
+			tftp_filename[MAX_LEN-1] = 0;
+		} else {
+			*p++ = '\0';
+			TftpServerIP = string_to_ip (BootFile);
+			strncpy(tftp_filename, p, MAX_LEN);
+			tftp_filename[MAX_LEN-1] = 0;
+		}
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
-	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t OurNet	= NetOurIP    & NetOurSubnetMask;
+	    IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
@@ -522,7 +541,7 @@ TftpStart (void)
 	/* Revert TftpBlkSize to dflt */
 	TftpBlkSize = TFTP_BLOCK_SIZE;
 #ifdef CONFIG_MCAST_TFTP
-    	mcast_cleanup();
+	mcast_cleanup();
 #endif
 
 	TftpSend ();
-- 
1.5.3.7

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip
add CONFIG_TFTP_FILE_NAME_MAX_LEN to configure the file name length
if not defined the max length will be at 128

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..b86ca86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..3dd2b06 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -55,7 +55,14 @@ static int	TftpState;
 
 #define DEFAULT_NAME_LEN	(8 + 4 + 1)
 static char default_filename[DEFAULT_NAME_LEN];
-static char *tftp_filename;
+
+#ifndef CONFIG_TFTP_FILE_NAME_MAX_LEN
+#define MAX_LEN 128
+#else
+#define MAX_LEN CONFIG_TFTP_FILE_NAME_MAX_LEN
+#endif
+
+static char tftp_filename[MAX_LEN];
 
 #ifdef CFG_DIRECT_FLASH_TFTP
 extern flash_info_t flash_info[];
@@ -231,7 +238,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -372,7 +379,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
 #ifdef CONFIG_MCAST_TFTP
 		/* if I am the MasterClient, actively calculate what my next
 		 * needed block is; else I'm passive; not ACKING
- 		 */
+		 */
 		if (Multicast) {
 			if (len < TftpBlkSize)  {
 				TftpEndingBlock = TftpBlock;
@@ -453,30 +460,43 @@ TftpStart (void)
 	char *ep;             /* Environment pointer */
 #endif
 
+	TftpServerIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
 			(NetOurIP >>  8) & 0xFF,
 			(NetOurIP >> 16) & 0xFF,
 			(NetOurIP >> 24) & 0xFF	);
-		tftp_filename = default_filename;
+
+		strncpy(tftp_filename, default_filename, MAX_LEN);
+		tftp_filename[MAX_LEN-1] = 0;
 
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p = strchr (p, ':');
+
+		if (p == NULL) {
+			strncpy(tftp_filename, BootFile, MAX_LEN);
+			tftp_filename[MAX_LEN-1] = 0;
+		} else {
+			*p++ = '\0';
+			TftpServerIP = string_to_ip (BootFile);
+			strncpy(tftp_filename, p, MAX_LEN);
+			tftp_filename[MAX_LEN-1] = 0;
+		}
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
-	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t OurNet	= NetOurIP    & NetOurSubnetMask;
+	    IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
@@ -522,7 +542,7 @@ TftpStart (void)
 	/* Revert TftpBlkSize to dflt */
 	TftpBlkSize = TFTP_BLOCK_SIZE;
 #ifdef CONFIG_MCAST_TFTP
-    	mcast_cleanup();
+	mcast_cleanup();
 #endif
 
 	TftpSend ();
-- 
1.5.3.7

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Wolfgang Denk]

In message <1200615243-5681-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> allow to use a different server as set in serverip
> add CONFIG_TFTP_FILE_NAME_MAX_LEN to configure the file name length
> if not defined the max length will be at 128
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"And it should be the law: If you use  the  word  `paradigm'  without
knowing  what  the  dictionary  says  it  means,  you  go to jail. No
exceptions."                     - David Jones @ Megatest Corporation

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Ben Warren]

Wolfgang,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> allow to use a different server as set in serverip
> add CONFIG_TFTP_FILE_NAME_MAX_LEN to configure the file name length
> if not defined the max length will be at 128
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>   
Acked-by: Ben Warren <biggerbadderben@gmail.com>

I'm not fussy whether this version or a modified Andre Renaud version 
goes in. Please pick one and apply directly.

regards,
Ben

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Wolfgang Denk]

In message <1199914268-29217-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> allow to use a different server as set in serverip
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index 21682c0..e03ffbf 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	tftpboot,	3,	1,	do_tftpb,
>  	"tftpboot- boot image via network using TFTP protocol\n",
> -	"[loadAddress] [bootfilename]\n"
> +	"[loadAddress] [[host ip addr]:bootfilename]\n"

NAK.

Ben, please don't apply. I reject this patch.

Jean-Christophe - as already discussed on IRC I really ask you to pay
attemtion to gvb's comments on your patch:

| Critique: the help is misleading: "host ip addr:" is optional so it 
| needs another set of [] around it.  Having spaces in "host ip addr" is 
| more readable, but makes it look like it is three things or perhaps a 
| keyword.
| 
| I would suggest the following, but there may be a better way...
| 	"[loadAddress] [[hostIPaddr:]bootfilename]\n"

Also make sure that you place the ';' in the right pair of brackets.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The X11 source code style is ATROCIOUS and should not be used  as  a
model."                                                   - Doug Gwyn

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..4e3d489 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[host ip addr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..d5d6e65 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -231,7 +231,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -464,19 +464,27 @@ TftpStart (void)
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p=BootFile;
+		p = strchr (p, ':');
+		if (p != NULL) {
+			TftpServerIP = string_to_ip (BootFile);
+			++p;
+			strcpy (tftp_filename, p);
+		} else {
+			strcpy (tftp_filename, BootFile);
+		}
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t ServerNet 	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 4e3d489..fec87b6 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	nfs,	3,	1,	do_nfs,
 	"nfs\t- boot image via network using NFS protocol\n",
-	"[loadAddress] [host ip addr:bootfilename]\n"
+	"[loadAddress] [[host ip addr:]bootfilename]\n"
 );
 #endif
 
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Wolfgang Denk]

In message <1199958365-18516-2-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index 4e3d489..fec87b6 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	nfs,	3,	1,	do_nfs,
>  	"nfs\t- boot image via network using NFS protocol\n",
> -	"[loadAddress] [host ip addr:bootfilename]\n"
> +	"[loadAddress] [[host ip addr:]bootfilename]\n"

Rejected. Please see previous reject message for the reason.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the beginning, there was nothing, which exploded.
                                - Terry Pratchett, _Lords and Ladies_

[U-Boot-Users] [PATCH] TFTP: add host ip addr support

[Jerry Van Baren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index 21682c0..2fb0e7c 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	tftpboot,	3,	1,	do_tftpb,
>  	"tftpboot- boot image via network using TFTP protocol\n",
> -	"[loadAddress] [bootfilename]\n"
> +	"[loadAddress] [host ip addr:bootfilename]\n"

Hi Jean-Christophe,

Interesting.  I can see how that would be useful rather than having to 
modify the dhcp server configuration when you want to use an alternate 
server.

Critique: the help is misleading: "host ip addr:" is optional so it 
needs another set of [] around it.  Having spaces in "host ip addr" is 
more readable, but makes it look like it is three things or perhaps a 
keyword.

I would suggest the following, but there may be a better way...
	"[loadAddress] [[hostIPaddr:]bootfilename]\n"

Best regards,
gvb

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..e03ffbf 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[host ip addr]:bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..d5d6e65 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -231,7 +231,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -464,19 +464,27 @@ TftpStart (void)
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p=BootFile;
+		p = strchr (p, ':');
+		if (p != NULL) {
+			TftpServerIP = string_to_ip (BootFile);
+			++p;
+			strcpy (tftp_filename, p);
+		} else {
+			strcpy (tftp_filename, BootFile);
+		}
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t ServerNet 	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index e03ffbf..894d863 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	nfs,	3,	1,	do_nfs,
 	"nfs\t- boot image via network using NFS protocol\n",
-	"[loadAddress] [host ip addr:bootfilename]\n"
+	"[loadAddress] [[host ip addr]:bootfilename]\n"
 );
 #endif
 
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Ben Warren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index e03ffbf..894d863 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	nfs,	3,	1,	do_nfs,
>  	"nfs\t- boot image via network using NFS protocol\n",
> -	"[loadAddress] [host ip addr:bootfilename]\n"
> +	"[loadAddress] [[host ip addr]:bootfilename]\n"
>  );
>  #endif
>  
>   
Sorry for being a PITA, but since we're changing documentation shouldn't 
this be:

-	"[loadAddress] [host ip addr:bootfilename]\n"
+	"[loadAddress] [[host ip addr:]bootfilename]\n"


Can you spot the difference?

regards,
Ben

 

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Wolfgang Denk]

In message <47854A1E.5060704@gmail.com> you wrote:
>
> > -	"[loadAddress] [host ip addr:bootfilename]\n"
> > +	"[loadAddress] [[host ip addr]:bootfilename]\n"
> >  );
> >  #endif
> >  
> >   
> Sorry for being a PITA, but since we're changing documentation shouldn't 
> this be:

You're not a PITA, but a careful reader.

> -	"[loadAddress] [host ip addr:bootfilename]\n"
> +	"[loadAddress] [[host ip addr:]bootfilename]\n"
> 
> 
> Can you spot the difference?

Indeed - it's the difference between right and wrong, between working
and failing.

Also, I ask to change this (as gvb previously recommended) into

	"[loadAddress] [[HostIpAddr:]bootfilename]\n"


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
EMACS belongs in <sys/errno.h>: Editor too big!

[U-Boot-Users] [PATCH 0/1] TFTP: add host ip addr support

[Jean-Christophe PLAGNIOL-VILLARD]

allow to use a different server as set in serverip

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 21682c0..b86ca86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	tftpboot,	3,	1,	do_tftpb,
 	"tftpboot- boot image via network using TFTP protocol\n",
-	"[loadAddress] [bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
diff --git a/net/tftp.c b/net/tftp.c
index 8b95bcf..d5d6e65 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -34,7 +34,7 @@
 #define TFTP_ERROR	5
 #define TFTP_OACK	6
 
-
+static IPaddr_t TftpServerIP;
 static int	TftpServerPort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
@@ -231,7 +231,7 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
 }
 
 
@@ -464,19 +464,27 @@ TftpStart (void)
 		printf ("*** Warning: no boot file name; using '%s'\n",
 			tftp_filename);
 	} else {
-		tftp_filename = BootFile;
+		char *p=BootFile;
+		p = strchr (p, ':');
+		if (p != NULL) {
+			TftpServerIP = string_to_ip (BootFile);
+			++p;
+			strcpy (tftp_filename, p);
+		} else {
+			strcpy (tftp_filename, BootFile);
+		}
 	}
 
 #if defined(CONFIG_NET_MULTI)
 	printf ("Using %s device\n", eth_get_name());
 #endif
-	puts ("TFTP from server ");	print_IPaddr (NetServerIP);
+	puts ("TFTP from server ");	print_IPaddr (TftpServerIP);
 	puts ("; our IP address is ");	print_IPaddr (NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 	    IPaddr_t OurNet 	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet 	= NetServerIP & NetOurSubnetMask;
+	    IPaddr_t ServerNet 	= TftpServerIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet) {
 		puts ("; sending through gateway ");
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/common/cmd_net.c b/common/cmd_net.c
index b86ca86..dbf6b86 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	nfs,	3,	1,	do_nfs,
 	"nfs\t- boot image via network using NFS protocol\n",
-	"[loadAddress] [host ip addr:bootfilename]\n"
+	"[loadAddress] [[hostIPaddr:]bootfilename]\n"
 );
 #endif
 
-- 
1.5.3.7

[U-Boot-Users] [PATCH 1/1] Fix nfs command help to reflect that the serverip is optional

[Wolfgang Denk]

In message <1199914268-29217-2-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index e03ffbf..894d863 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  U_BOOT_CMD(
>  	nfs,	3,	1,	do_nfs,
>  	"nfs\t- boot image via network using NFS protocol\n",
> -	"[loadAddress] [host ip addr:bootfilename]\n"
> +	"[loadAddress] [[host ip addr]:bootfilename]\n"
>  );
>  #endif

Rejected, because this is not correct:

=> print serverip
serverip=192.168.1.1
=> nfs 200000 :/opt/eldk/ppc_4xx/images/uImage
Using ppc_4xx_eth0 device
File transfer via NFS from server 0.0.0.0; our IP address is
192.168.80.2
Filename '/opt/eldk/ppc_4xx/images/uImage'.
Load address: 0x200000
Loading: *** ERROR: Cannot mount
...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Make it right before you make it faster.