This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: RedBoot Network support fix
- From: Gary Thomas <gary at mlbassoc dot com>
- To: Yoshinori Sato <ysato at users dot sourceforge dot jp>
- Cc: eCos patches <ecos-patches at ecos dot sourceware dot org>
- Date: Tue, 01 Feb 2005 10:43:35 -0700
- Subject: Re: RedBoot Network support fix
- Organization: MLB Associates
- References: <m24qh3wkjs.wl%ysato@users.sourceforge.jp>
On Thu, 2005-01-27 at 13:41 +0900, Yoshinori Sato wrote:
> I fix the handling of IP address
I have some questions about these changes.
>
> * src/main.c (cyg_start): Fix compiler warning with
> Index: packages/redboot/current/src/net/bootp.c
> ===================================================================
> RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/bootp.c,v
> retrieving revision 1.1.1.6
> retrieving revision 1.2
> diff -u -r1.1.1.6 -r1.2
> --- packages/redboot/current/src/net/bootp.c 4 Mar 2004 05:23:06 -0000 1.1.1.6
> +++ packages/redboot/current/src/net/bootp.c 27 Jan 2005 04:23:46 -0000 1.2
> @@ -135,6 +135,7 @@
> // The request message has been sent, only accept an ack reply
> if (*p == DHCP_MESS_TYPE_ACK) {
> dhcpState = DHCP_ACK;
> + bp_info->bp_siaddr = b->bp_siaddr;
> return;
> } else {
> expected = DHCP_MESS_TYPE_ACK;
What is this supposed to do? The default server is already being
obtained from the DHCP response (at least it works properly on all
the targets I've tested).
> Index: packages/redboot/current/src/net/http_client.c
> ===================================================================
> RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/http_client.c,v
> retrieving revision 1.1.1.7
> retrieving revision 1.7
> diff -u -r1.1.1.7 -r1.7
> --- packages/redboot/current/src/net/http_client.c 12 Jan 2005 02:42:44 -0000 1.1.1.7
> +++ packages/redboot/current/src/net/http_client.c 27 Jan 2005 04:20:51 -0000 1.7
> @@ -86,7 +86,12 @@
> struct _stream *s = &http_stream;
>
> if (!info->server->sin_port)
> - info->server->sin_port = 80; // HTTP port
> + info->server->sin_port = htons(80); // HTTP port
> + if (info->server->sin_addr.s_addr == 0) {
> + // server address 0.0.0.0
> + *err = HTTP_OPEN;
> + return -1;
> + }
> if ((res = __tcp_open(&s->sock, info->server, get_port++, 5000, err)) < 0) {
> *err = HTTP_OPEN;
> return -1;
This one seems OK.
> Index: packages/redboot/current/src/net/net_io.c
> ===================================================================
> RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/net_io.c,v
> retrieving revision 1.1.1.8
> retrieving revision 1.9
> diff -u -r1.1.1.8 -r1.9
> --- packages/redboot/current/src/net/net_io.c 23 Nov 2004 09:00:42 -0000 1.1.1.8
> +++ packages/redboot/current/src/net/net_io.c 27 Jan 2005 04:22:29 -0000 1.9
> @@ -678,6 +678,7 @@
> #endif
> #ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> char ip_addr[16];
> + struct in_addr default_addr;
> #endif
>
> // Set defaults as appropriate
> @@ -782,7 +783,9 @@
> #ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> diag_sprintf(ip_addr, "%d.%d.%d.%d",
> CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR);
> - inet_aton(ip_addr, &my_bootp_info.bp_siaddr);
> + inet_aton(ip_addr, &default_addr);
> + if (default_addr.s_addr)
> + my_bootp_info.bp_siaddr = default_addr;
> #endif
> #ifdef CYGSEM_REDBOOT_FLASH_CONFIG
> flash_get_IP("bootp_server_ip", (ip_addr_t *)&my_bootp_info.bp_siaddr);
This seems pointless. If the server IP address has been configured in,
it should be set to a legal, non-zero, value. All you've done is
duplicate the existing behaviour that if the default server is not set
(i.e. is 0.0.0.0) by fconfig, then get it from DHCP.
Please explain.
> Index: packages/redboot/current/src/net/tftp_client.c
> ===================================================================
> RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/tftp_client.c,v
> retrieving revision 1.1.1.9
> retrieving revision 1.2
> diff -u -r1.1.1.9 -r1.2
> --- packages/redboot/current/src/net/tftp_client.c 12 Jan 2005 02:42:44 -0000 1.1.1.9
> +++ packages/redboot/current/src/net/tftp_client.c 27 Jan 2005 04:21:33 -0000 1.2
> @@ -103,6 +103,12 @@
> tftp_stream.local_addr.sin_addr.s_addr = htonl(INADDR_ANY);
> tftp_stream.local_addr.sin_port = htons(get_port++);
>
> + if (info->server->sin_addr.s_addr == 0) {
> + // server address 0.0.0.0
> + *err = TFTP_EBADOP;
> + return -1;
> + }
> +
> if (info->server->sin_port == 0) {
> info->server->sin_port = htons(TFTP_PORT);
> } else {
> ============================================================
This one seems OK as well.
Once I understand the rationale behind these changes, I'll apply
them as appropriate to the source tree.
Thanks
--
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------