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

newer
[U-Boot-Users] [PATCH] ARM926:...

older
[U-Boot-Users] [PATCH] ARM:...

Jean-Christophe PLAGNIOL-VILLARD

12 Jan 2008 12 Jan '08

12:43 a.m.

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

Show replies by date

Jean-Christophe PLAGNIOL-VILLARD

15 Jan 15 Jan

2:50 p.m.

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.

Ben Warren

16 Jan 16 Jan

4:19 a.m.

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

Jean-Christophe PLAGNIOL-VILLARD

8:19 a.m.

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.

Ben Warren

4:49 p.m.

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

Jean-Christophe PLAGNIOL-VILLARD

6:33 p.m.

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.

Ben Warren

9:27 p.m.

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

Wolfgang Denk

9:40 p.m.

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@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.

Ben Warren

9:47 p.m.

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

McMullan, Jason

9:56 p.m.

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.

Wolfgang Denk

10:26 p.m.

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@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@denx.de A committee is a group that keeps the minutes and loses hours. -- Milton Berle

Jean-Christophe PLAGNIOL-VILLARD

11:27 p.m.

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@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.

Wolfgang Denk

17 Jan 17 Jan

12:09 a.m.

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@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

Ben Warren

12:33 a.m.

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

Jean-Christophe PLAGNIOL-VILLARD

11:08 p.m.

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

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

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

#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

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; + } }

/* 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

Jean-Christophe PLAGNIOL-VILLARD

11:08 p.m.

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

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

Ben Warren

11:29 p.m.

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

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

Wolfgang Denk

11:31 p.m.

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

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@denx.de Vulcans worship peace above all. -- McCoy, "Return to Tomorrow", stardate 4768.3

Andre Renaud

11:42 p.m.

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

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

- +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

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 (OurNet != ServerNet) { puts ("; sending through gateway ");

Ben Warren

18 Jan 18 Jan

1:19 a.m.

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

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

Jean-Christophe PLAGNIOL-VILLARD

1:30 a.m.

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

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.

Wolfgang Denk

16 Jan 16 Jan

10:23 p.m.

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@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.

Ben Warren

10:38 p.m.

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

Jean-Christophe PLAGNIOL-VILLARD

18 Jan 18 Jan

12:51 a.m.

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

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

#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

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

Jean-Christophe PLAGNIOL-VILLARD

1:14 a.m.

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

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

#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

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

Wolfgang Denk

5 Feb 5 Feb

12:42 a.m.

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@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

Ben Warren

18 Jan 18 Jan

1:23 a.m.

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

6301

Age (days ago)

6325

Last active (days ago)

List overview

Download

26 comments

5 participants

tags (0)

participants (5)

Andre Renaud
Ben Warren
Jean-Christophe PLAGNIOL-VILLARD
McMullan, Jason
Wolfgang Denk