>
> please make var 'iface' local and while you are there, move
> all the 'local' declaration to the head of the function.

Wouldn't be better if the local vars inside of the if and for statements
stay inside?
I mean what is the point of declaring ifaces if I will be using all the
ifaces? in term of performance(even if it is negligible and clarity of the
code).

Thanks for following the patch!
Regards,
Amine

On Thu, Jan 14, 2016 at 10:44 AM, Bastian Bittorf <bitt...@bluebottle.com>
wrote:

> * amine ahd <amine....@gmail.com> [14.01.2016 10:29]:
>
> thank you, patch applies...
>
> >  start_service() {
> > -     local server enabled enable_server peer
> > +     local server enabled enable_server peer ntpservers
> > +     local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
> >
> >       validate_ntp_section ntp || {
> >               echo "validation failed"
> > @@ -21,13 +25,33 @@ start_service() {
> >       }
> >
> >       [ $enabled = 0 ] && return
> > -
> > -     [ -z "$server" ] && return
> > +     [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return
>
> i'am ok with this, if you like you can reuse
> 'config_get_bool()' from /lib/functions.sh
>
> >       procd_open_instance
> >       procd_set_param command "$PROG" -n
> >       [ "$enable_server" = "1" ] && procd_append_param command -l
> >       [ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S
> "$HOTPLUG_SCRIPT"
> > +
> > +     local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
> > +     [ "$use_dhcp" = 1 ] && {
>
> this should also be 'bool'
>
> > +             if [ -z "$dhcp_ifaces" ]; then
> > +                     local dump="$(ubus call network.interface dump)"
> > +                     ntpservers=$(jsonfilter -s "$dump" -e
> '$["interface"][*]["data"]["ntpserver"]')
> > +             else
> > +                     for iface in $dhcp_ifaces; do
>
> please make var 'iface' local and while you are there, move
> all the 'local' declaration to the head of the function.
>
> the rest looks OK to me. - thank you
>
> bye, bastian
>



-- 

Amine Hamed | Software Engineer



Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany

Email aha...@ocedo.com


<aha...@ocedo.com>

REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER:
HRB 717873
MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Reply via email to