Bug#362029: Patch to attach

[Ferenc Wagner]

Matthew Palmer mpal...@debian.org writes:

 Index: netcfg/dhcp.c
 ===
 --- netcfg/dhcp.c (revision 66154)
 +++ netcfg/dhcp.c (working copy)
 @@ -521,6 +521,7 @@ (after udhcpc exited with a lease)
  }
  
  netcfg_nameservers_to_array (nameservers, 
 nameserver_array);
 +netcfg_write_resolv (domain, nameserver_array);
  }
  
  state = HOSTNAME;
 @@ -602,6 +603,16 @@ (in case DOMAIN:)
  else {
  netcfg_write_common(ipaddress, hostname, domain);
  netcfg_write_dhcp(interface, dhostname);
 +/* If the resolv.conf was written by udhcpc, then 
 nameserver_array
 + * will be empty and we'll need to populate it.  If we asked 
 for
 + * the nameservers, then it'll be full, but nobody will care 
 if we
 + * refill it.
 + */
 +if (read_resolv_conf_nameservers(nameserver_array))
 +netcfg_write_resolv(domain, nameserver_array);
 +else
 +printf(Error reading resolv.conf for nameservers\n);
 +
  return 0;
  }
  break;

Hi,

This seems to do the job, but the control flow in netcfg is already
convoluted enough, isn't there a better alternative?  I mean, for a
static configuration, netcfg_write_resolv is called from
netcfg_write_static, can't we follow suit in netcfg_write_dhcp?  If
netcfg always removed resolv.conf before calling the DHCP client, then
we could modify resolv_conf_entries to also populate nameserver_array, 
and then unconditionally write out resolv.conf in netcfg_write_dhcp,
couldn't we?  Just thinking out loud

 @@ -646,3 +657,41 @@
  
  return count;
  }
 +
 +/* Read the nameserver entries out of resolv.conf and stick them into
 + * nameservers_array, so we can write out a newer, shinier resolv.conf
 + */
 +int read_resolv_conf_nameservers(struct in_addr array[])
 +{
 +FILE *f;
 +int i = 0;
 +
 +if ((f = fopen(RESOLV_FILE, r)) != NULL) {
 +char buf[256];
 +
 +while (fgets(buf, 256, f) != NULL) {
 +char *ptr;
 +
 +if (strncmp(buf, nameserver , strlen(nameserver )) == 0) {
 +/* Chop off trailing \n */
 +while (buf[strlen(buf)-1] == '\n')
 +buf[strlen(buf)-1] = '\0';

Why is this while instead of if?

 +ptr = buf + strlen(nameserver );
 +inet_pton(AF_INET, ptr, array[i++]);
 +if (i == 3) {

We really should use a global constant here (and in netcfg.h and in
netcfg_nameservers_to_array at least) instead of the magic number 3...

 +/* We can only hold so many nameservers, and we've 
 reached
 + * our limit.  Sorry.
 + */
-- 
Regards,
Feri.



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#362029: Patch to attach

[Matthew Palmer]

Le sigh...

- Matt
Index: netcfg/dhcp.c
===
--- netcfg/dhcp.c	(revision 66154)
+++ netcfg/dhcp.c	(working copy)
@@ -521,6 +521,7 @@
 }
 
 netcfg_nameservers_to_array (nameservers, nameserver_array);
+netcfg_write_resolv (domain, nameserver_array);
 }
 
 state = HOSTNAME;
@@ -602,6 +603,16 @@
 else {
 netcfg_write_common(ipaddress, hostname, domain);
 netcfg_write_dhcp(interface, dhostname);
+/* If the resolv.conf was written by udhcpc, then nameserver_array
+ * will be empty and we'll need to populate it.  If we asked for
+ * the nameservers, then it'll be full, but nobody will care if we
+ * refill it.
+ */
+if (read_resolv_conf_nameservers(nameserver_array))
+netcfg_write_resolv(domain, nameserver_array);
+else
+printf(Error reading resolv.conf for nameservers\n);
+
 return 0;
 }
 break;
@@ -646,3 +657,41 @@
 
 return count;
 }
+
+/* Read the nameserver entries out of resolv.conf and stick them into
+ * nameservers_array, so we can write out a newer, shinier resolv.conf
+ */
+int read_resolv_conf_nameservers(struct in_addr array[])
+{
+FILE *f;
+int i = 0;
+
+if ((f = fopen(RESOLV_FILE, r)) != NULL) {
+char buf[256];
+
+while (fgets(buf, 256, f) != NULL) {
+char *ptr;
+
+if (strncmp(buf, nameserver , strlen(nameserver )) == 0) {
+/* Chop off trailing \n */
+while (buf[strlen(buf)-1] == '\n')
+buf[strlen(buf)-1] = '\0';
+
+ptr = buf + strlen(nameserver );
+inet_pton(AF_INET, ptr, array[i++]);
+if (i == 3) {
+/* We can only hold so many nameservers, and we've reached
+ * our limit.  Sorry.
+ */
+break;
+}
+}
+}
+
+fclose(f);
+array[i].s_addr = 0;
+return 1;
+}
+else
+return 0;
+}
Index: netcfg/netcfg.h
===
--- netcfg/netcfg.h	(revision 66154)
+++ netcfg/netcfg.h	(working copy)
@@ -98,6 +98,8 @@
 
 extern int resolv_conf_entries (void);
 
+extern int read_resolv_conf_nameservers (struct in_addr array[]);
+
 extern int ask_dhcp_options (struct debconfclient *client);
 extern int netcfg_activate_static(struct debconfclient *client);
 

Bug#362029: Patch to attach

[Christian PERRIER]

Quoting Matthew Palmer (mpal...@debian.org):
 Le sigh...
 
 - Matt


:-)

I added this bug the RC1 blockers 
(http://wiki.debian.org/DebianInstaller/ReleaseAnnounce). Given that you 
confirmed that you
carefully tested this patch, I'm ready to commit it and rebuild
netcfg...unless other team members have objections to this.

Thnaks for your work investigating this (and doing some cleaning work
in netcfg bugs).




signature.asc
Description: Digital signature