This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: RedBoot Network support fix
- From: Yoshinori Sato <ysato at users dot sourceforge dot jp>
- To: Gary Thomas <gary at mlbassoc dot com>
- Cc: Yoshinori Sato <ysato at users dot sourceforge dot jp>,eCos patches <ecos-patches at ecos dot sourceware dot org>
- Date: Mon, 07 Feb 2005 15:48:40 +0900
- Subject: Re: RedBoot Network support fix
- References: <m24qh3wkjs.wl%ysato@users.sourceforge.jp><1107279815.18185.110.camel@hermes><m2651ai6hz.wl%ysato@users.sourceforge.jp><1107427767.18185.247.camel@hermes>
At Thu, 03 Feb 2005 03:49:27 -0700,
Gary Thomas wrote:
>
> On Thu, 2005-02-03 at 13:59 +0900, Yoshinori Sato wrote:
> > At Tue, 01 Feb 2005 10:43:35 -0700,
> > Gary Thomas wrote:
> > >
> > > 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).
> >
> > What I put by testing stayed.
> > This is useless.
> >
> > > > 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.
> >
> > Because CYGDAT_REDBOOT_BOOTP_SERVER_IP_ADDR is defined by "0,0,0,0" when do not use fconfig,
> > default server which got with dhcp is destroyed.
>
> My point is that CYGDAT_REDBOOT_BOOTP_SERVER_IP_ADDR should only be
> used to define a valid server IP address.
I ignored it in case of "0, 0, 0, 0".
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
diff -u -r1.1.1.8 net_io.c
--- 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 4 Feb 2005 15:31:00 -0000
@@ -174,6 +174,9 @@
#define TELNET_DO 0xFD // Will you XXX
#define TELNET_TM 0x06 // Time marker (special DO/WONT after IP)
+#define IP_ADDR(ADDR) _IP_ADDR(ADDR)
+#define _IP_ADDR(a, b, c, d) ((a << 24) | (b << 16) | (c << 8) | d)
+
static cyg_bool
_net_io_getc_nonblock(void* __ch_data, cyg_uint8* ch)
{
@@ -676,7 +679,8 @@
char *default_devname;
int default_index;
#endif
-#ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
+#if defined(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) && \
+ IP_ADDR(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) != 0
char ip_addr[16];
#endif
@@ -779,7 +783,8 @@
}
if (have_net) {
show_eth_info();
-#ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
+#if defined(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) && \
+ IP_ADDR(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) != 0
diag_sprintf(ip_addr, "%d.%d.%d.%d",
CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR);
inet_aton(ip_addr, &my_bootp_info.bp_siaddr);
--
Yoshinori Sato
<ysato@users.sourceforge.jp>