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

Wolfgang Denk
2008-01-17T22:31:26Z

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