[OpenWrt-Devel] [PATCH 1/1] [DEV-1329] use NTP server received via DHCP

[Bastian Bittorf]

* amine ahd <amine.ahd at gmail.com> [07.01.2016 10:34]:

the patch is from wrong dir.
please do a 'git format-patch' inside the OpenWrt-dir,
so the modified files are:

package/utils/busybox/Makefile
package/utils/busybox/files/sysntpd
package/utils/busybox/files/sysntpd.hotplug

for the subject: what means "[DEV-1329]"?

> +. /usr/share/libubox/jshn.sh
>  START=98
>  
>  USE_PROCD=1
> @@ -22,12 +24,32 @@ start_service() {
>  
>  	[ $enabled = 0 ] && return
>  
> -	[ -z "$server" ] && return

please check if any interface is in DHCP-mode
and has a chance to get an NTP, otherwise return. 

> +	if [ "$use_dhcp" = 1 ]; then

minor: use OpenWrt-style:
when there is no 'else', just do:

[ "$use_dhcp" = 1 ] && {
	...
}

> +		if [ -z "$dhcp_ifaces" ]; then
> +			dump=$(ubus call network.interface dump)

make 'dump' also 'local'

> +check_int() {

minor: choose better function name.

> +	list=$(uci get system.ntp.dhcp_ifaces)
> +	if [ -z $list ];
> +	then
> +		return 0
> +	fi

it's shorter:
[ -z "$list" ] && return

> +	if [ "${list#*$INTERFACE}" != "$list" ]

this looks strange to me and will IMHO not work
for similar names, e.g. eth0 eth0.1 eth0.2

you want to test, if the upcoming $INTERFACE is part
of allowed interfaces ("system.ntp.dhcp_ifaces"), aren't you?

is_valid_interface()
{
	local list="$(uci get system.ntp.dhcp_ifaces)"

	case " $list " in
		*" $INTERFACE "*)
		;;
		*)
			return 1
		;;
	esac
}

> +	for int in $dhcp_ifaces; do

please you 'iface' or 'interface' not 'int'
but thanks for the patch for now!

bye, bastian
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel