[U-Boot-Users] [PATCH] fixed tftp error message output
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
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
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
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
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
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
participants (2)
-
Andreas Oberritter -
Wolfgang Denk