Christian Gmeiner dixit:
>I have here a patch, which fixes the following thing. I have a static
>WAN interface and at the moment it does not run out-of-the-box. This
>means I need to add my nameservers to /etc/resolv.conf a) by hand b)
>by modifying some files.
The intent is good, but I have a little criticism on the patch.
This:
> Index: package/base-files/netcfg/interfaces.static.resolver
> ===================================================================
> --- package/base-files/netcfg/interfaces.static.resolver (revision 0)
> +++ package/base-files/netcfg/interfaces.static.resolver (revision 0)
> @@ -0,0 +1,2 @@
> + resolver @FWRT_RESOLVER@
> +
> Index: package/base-files/Makefile
> ===================================================================
> --- package/base-files/Makefile (revision 3555)
> +++ package/base-files/Makefile (working copy)
> @@ -186,6 +186,12 @@
> ed -s ${IDIR_BASE_FILES}/etc/network/interfaces
> endif
> endif
> +ifneq ($(strip $(subst ",,${FWRT_NETWORK_WAN_IP_RESOLVER})),)
> + cat ./netcfg/interfaces.static.resolver \
> + >>$(IDIR_BASE_FILES)/etc/network/interfaces
> + $(SED) "s/@FWRT_RESOLVER@/$(strip $(subst
> ",,${FWRT_NETWORK_WAN_IP_RESOLVER}))/g" \
> + $(IDIR_BASE_FILES)/etc/network/interfaces
> +endif
> ifeq ($(FWRT_NETWORK_WLAN),y)
> printf '\n%s\n' "# WLAN configuration"
> >>$(IDIR_BASE_FILES)/etc/network/interfaces
> ifeq ($(FWRT_NETWORK_LANBRIDGE),y)
First off, wouldn't it be *much* easier to do something like this:
+ ifneq ($(strip $(subst ",,${FWRT_NETWORK_WAN_IP_RESOLVER})),)
+ echo "resolver $(strip $(subst ",,${FWRT_NETWORK_WAN_IP_RESOLVER}))" \
+ >>$(IDIR_BASE_FILES)/etc/network/interfaces
+ endif
Second, a little criticism:
> - ${INSTALL_DATA} ${PKG_SRC_ROOT}/lib/lladdr
> ${IDIR_FWIFUPDOWN}/${PKG_CONFIG_PLUGDIR}/
> + ${INSTALL_DATA} ${PKG_SRC_ROOT}/lib/lladdr
> ${IDIR_FWIFUPDOWN}/${PKG_CONFIG_PLUGDIR}
We use syntax like this:
${INSTALL_DIR} directoryname
and then either
${INSTALL_DATA} filename directoryname/newfilename
${INSTALL_DATA} filename directoryname/
The trailing slash is important so that, if we accidentally forget
to create the directory first, end up with an error.
Lastly, even with these changes, I think your diff is too
complicated. How about this one?
Index: package/Config.in.network
===================================================================
--- package/Config.in.network (revision 3555)
+++ package/Config.in.network (working copy)
@@ -196,6 +196,16 @@
depends on FWRT_NETWORK_WAN_PROTO_STATIC
help
IP gateway
+
+config FWRT_NETWORK_NAMESERVERS
+ string "DNS nameservers"
+ default ""
+ depends on FWRT_NETWORK_WAN_PROTO_STATIC
+ help
+ As static configuration of wan interface is choosen
+ it is possible to define which resolvers (DNS servers)
+ should be used. Leave blank if you don't want to use a
+ name server, separate with spaces if you want two.
endmenu
menu "Wireless LAN"
Index: package/base-files/Makefile
===================================================================
--- package/base-files/Makefile (revision 3555)
+++ package/base-files/Makefile (working copy)
@@ -186,6 +186,12 @@
ed -s ${IDIR_BASE_FILES}/etc/network/interfaces
endif
endif
+ifneq ($(strip $(subst ",,${FWRT_NETWORK_NAMESERVERS})),)
+ -rm -f ${IDIR_BASE_FILES}/etc/resolv.conf # the symlink
+ for nameserver in $(strip $(subst ",,${FWRT_NETWORK_NAMESERVERS})); do \
+ echo "nameserver $$nameserver"; \
+ done >${IDIR_BASE_FILES}/etc/resolv.conf # the file
+endif
ifeq ($(FWRT_NETWORK_WLAN),y)
printf '\n%s\n' "# WLAN configuration"
>>$(IDIR_BASE_FILES)/etc/network/interfaces
ifeq ($(FWRT_NETWORK_LANBRIDGE),y)
That’s all to it, there is no need to change the format of
/etc/network/interfaces again for it.
What do you think of this approach/solution?
>PS: Have a look at http://www.freewrt.org/trac/ticket/286 too.
I’d vote against it too (besides, dns1/dns2 is a mis-nomer,
you'd rather want dnsN where N is 1..9 or so).
bye,
//mirabilos
--
I believe no one can invent an algorithm. One just happens to hit upon it
when God enlightens him. Or only God invents algorithms, we merely copy them.
If you don't believe in God, just consider God as Nature if you won't deny
existence. -- Coywolf Qi Hunt
_______________________________________________
freewrt-developers mailing list
[email protected]
https://www.freewrt.org/lists/listinfo/freewrt-developers
