All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot-Users] [PATCH] fixed tftp error message output
@ 2003-07-19  2:34 Andreas Oberritter
  2003-07-19 19:28 ` Wolfgang Denk
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Oberritter @ 2003-07-19  2:34 UTC (permalink / raw)
  To: u-boot

This patch fixes tftp error message output, i.e. does not print the last
two bytes which contain garbage (at least for my setup, I hope this is
not a tftp server issue).

Changelog:

Patch by Andreas Oberritter, 19 July 03
 - fixed tftp error message output
-------------- next part --------------
A non-text attachment was scrubbed...
Name: u-boot-0.4.0-tftp.diff
Type: text/x-patch
Size: 535 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20030719/601b7473/attachment.bin 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot-Users] [PATCH] fixed tftp error message output
  2003-07-19  2:34 [U-Boot-Users] [PATCH] fixed tftp error message output Andreas Oberritter
@ 2003-07-19 19:28 ` Wolfgang Denk
  2003-07-19 20:28   ` Andreas Oberritter
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2003-07-19 19:28 UTC (permalink / raw)
  To: u-boot

Dear Andreas,

in message <1058582070.943.37.camel@localhost> you wrote:
> 
> This patch fixes tftp error message output, i.e. does not print the last
> two bytes which contain garbage (at least for my setup, I hope this is
> not a tftp server issue).

Is there a way to provoke such an error, so we can test this?

>  	case TFTP_ERROR:
> -		printf ("\nTFTP error: '%s' (%d)\n",
> -					pkt + 2, ntohs(*(ushort *)pkt));
> +		printf ("\nTFTP error %d: ", ntohs(*(ushort *)pkt));
> +		pkt += 2;
> +		len -= 2;
> +		while (len--)
> +			printf("%c", *pkt++);
> +		printf("\n");
> +

Patch rejected. What happens if "len" turns out to be zero?
Also, the code is unnecessary complex.

If there really is such a problem, this should do as well (and  maybe
better):

	printf ("\nTFTP error: '%.*s' (%d)\n",
		len - 2,
		pkt + 2,
		ntohs(*(ushort *)pkt) );

Can you please test this (and eventually re-submit the patch) ?

Thanks.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
A memorandum is written not to inform the reader, but to protect  the
writer.                                               -- Dean Acheson

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot-Users] [PATCH] fixed tftp error message output
  2003-07-19 19:28 ` Wolfgang Denk
@ 2003-07-19 20:28   ` Andreas Oberritter
  2003-07-19 21:59     ` Wolfgang Denk
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Oberritter @ 2003-07-19 20:28 UTC (permalink / raw)
  To: u-boot

On Sat, 2003-07-19 at 21:28, Wolfgang Denk wrote:
> > This patch fixes tftp error message output, i.e. does not print the last
> > two bytes which contain garbage (at least for my setup, I hope this is
> > not a tftp server issue).
> 
> Is there a way to provoke such an error, so we can test this?

Just try to tftp a file which does not exist on the server or which does
not have correct permissions (leading to a "file not found" message). I
am using atftpd on debian sarge and sid.

> What happens if "len" turns out to be zero?

Ok, I'll add a check for it.

> If there really is such a problem, this should do as well (and  maybe
> better):
> 
> 	printf ("\nTFTP error: '%.*s' (%d)\n",
> 		len - 2,
> 		pkt + 2,
> 		ntohs(*(ushort *)pkt) );
> 
> Can you please test this (and eventually re-submit the patch) ?

I see, this would indeed be simpler. I didn't know %.*s until now. I
will try it when I come home next week.

Regards,
Andreas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot-Users] [PATCH] fixed tftp error message output
  2003-07-19 20:28   ` Andreas Oberritter
@ 2003-07-19 21:59     ` Wolfgang Denk
  2003-07-19 22:31       ` Andreas Oberritter
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2003-07-19 21:59 UTC (permalink / raw)
  To: u-boot

In message <1058646514.774.55.camel@localhost> you wrote:
> 
> Just try to tftp a file which does not exist on the server or which does
> not have correct permissions (leading to a "file not found" message). I
> am using atftpd on debian sarge and sid.

Done - I get this then:

=> tftp 100000 foo
TFTP from server 10.0.0.14; our IP address is 10.0.0.99
Filename 'foo'.
Load address: 0x100000
Loading: *
TFTP error: 'File not found' (1)
Starting again


Nothing wrong, or am I missing something?


> > Can you please test this (and eventually re-submit the patch) ?
> 
> I see, this would indeed be simpler. I didn't know %.*s until now. I
> will try it when I come home next week.

Thanks!


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Computers are not intelligent. They only think they are.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot-Users] [PATCH] fixed tftp error message output
  2003-07-19 21:59     ` Wolfgang Denk
@ 2003-07-19 22:31       ` Andreas Oberritter
  2003-07-20 12:53         ` Wolfgang Denk
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Oberritter @ 2003-07-19 22:31 UTC (permalink / raw)
  To: u-boot

On Sat, 2003-07-19 at 23:59, Wolfgang Denk wrote:
> In message <1058646514.774.55.camel@localhost> you wrote:
> > 
> > Just try to tftp a file which does not exist on the server or which does
> > not have correct permissions (leading to a "file not found" message). I
> > am using atftpd on debian sarge and sid.
> 
> Done - I get this then:
> 
> => tftp 100000 foo
> TFTP from server 10.0.0.14; our IP address is 10.0.0.99
> Filename 'foo'.
> Load address: 0x100000
> Loading: *
> TFTP error: 'File not found' (1)
> Starting again
> 
> 
> Nothing wrong, or am I missing something?

My guess is that your tftp server correctly appends \0 to the error
message and atftpd does not. Strange. Having a look at atftpd source
code I suspect that its use of strncpy for the error message is the
cause, but I think u-boot should not rely on the trailing \0, because
the length of the error message is known anyway. I'll report when I have
been able to test it.

Regards,
Andreas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot-Users] [PATCH] fixed tftp error message output
  2003-07-19 22:31       ` Andreas Oberritter
@ 2003-07-20 12:53         ` Wolfgang Denk
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfgang Denk @ 2003-07-20 12:53 UTC (permalink / raw)
  To: u-boot

In message <1058653888.774.80.camel@localhost> you wrote:
>
> > Nothing wrong, or am I missing something?
> 
> My guess is that your tftp server correctly appends \0 to the error
> message and atftpd does not. Strange. Having a look at atftpd source
> code I suspect that its use of strncpy for the error message is the
> cause, but I think u-boot should not rely on the trailing \0, because
> the length of the error message is known anyway. I'll report when I have
> been able to test it.

The TFTP protocol explicitely requires the error string  to  be  '\0'
terminated:

Quote from RFC 1350 "THE TFTP PROTOCOL (REVISION 2)":

   ...
               2 bytes     2 bytes      string    1 byte
               -----------------------------------------
              | Opcode |  ErrorCode |   ErrMsg   |   0  |
               -----------------------------------------

                        Figure 5-4: ERROR packet

   An ERROR packet (opcode 5) takes the form depicted in Figure 5-4.  An
   ERROR packet can be the acknowledgment of any other type of packet.
   The error code is an integer indicating the nature of the error.  A
   table of values and meanings is given in the appendix.  (Note that
   several error codes have been added to this version of this
   document.) The error message is intended for human consumption, and
   should be in netascii.  Like all other strings, it is terminated with
   a zero byte.
   ...

IMHO the "it is terminated with a zero  byte"  is  mandatory,  so  no
change to U-Boot sems to be necessary.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Documentation is like sex: when it is good, it is  very,  very  good;
and when it is bad, it is better than nothing.         - Dick Brandon

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2003-07-20 12:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-19  2:34 [U-Boot-Users] [PATCH] fixed tftp error message output Andreas Oberritter
2003-07-19 19:28 ` Wolfgang Denk
2003-07-19 20:28   ` Andreas Oberritter
2003-07-19 21:59     ` Wolfgang Denk
2003-07-19 22:31       ` Andreas Oberritter
2003-07-20 12:53         ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.