<div dir="ltr"><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">please make var 'iface' local and while you are there, move<br>
all the 'local' declaration to the head of the function.</blockquote><div>Wouldn't be better if the local vars inside of the if and for statements stay inside?<br></div><div>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).<br><br></div><div>Thanks for following the patch!<br></div><div>Regards,<br></div><div>Amine <br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 14, 2016 at 10:44 AM, Bastian Bittorf <span dir="ltr"><<a href="mailto:bittorf@bluebottle.com" target="_blank">bittorf@bluebottle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">* amine ahd <<a href="mailto:amine.ahd@gmail.com">amine.ahd@gmail.com</a>> [14.01.2016 10:29]:<br>
<br>
thank you, patch applies...<br>
<span class=""><br>
> start_service() {<br>
> - local server enabled enable_server peer<br>
> + local server enabled enable_server peer ntpservers<br>
> + local use_dhcp="$(uci -q get system.ntp.use_dhcp)"<br>
><br>
> validate_ntp_section ntp || {<br>
> echo "validation failed"<br>
> @@ -21,13 +25,33 @@ start_service() {<br>
> }<br>
><br>
> [ $enabled = 0 ] && return<br>
> -<br>
> - [ -z "$server" ] && return<br>
> + [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return<br>
<br>
</span>i'am ok with this, if you like you can reuse<br>
'config_get_bool()' from /lib/functions.sh<br>
<span class=""><br>
> procd_open_instance<br>
> procd_set_param command "$PROG" -n<br>
> [ "$enable_server" = "1" ] && procd_append_param command -l<br>
> [ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"<br>
> +<br>
> + local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"<br>
> + [ "$use_dhcp" = 1 ] && {<br>
<br>
</span>this should also be 'bool'<br>
<span class=""><br>
> + if [ -z "$dhcp_ifaces" ]; then<br>
> + local dump="$(ubus call network.interface dump)"<br>
> + ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')<br>
> + else<br>
> + for iface in $dhcp_ifaces; do<br>
<br>
</span>please make var 'iface' local and while you are there, move<br>
all the 'local' declaration to the head of the function.<br>
<br>
the rest looks OK to me. - thank you<br>
<br>
bye, bastian<br>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:12px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline">Amine Hamed</span><span style="font-size:12px;font-family:Verdana;color:#ff850c;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"> | Software Engineer</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:12px;font-family:Verdana;color:#ff850c;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"><br></span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:11.333333333333332px;font-family:Verdana;color:#ff850c;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"> </span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:10px;font-family:Verdana;color:#ff9900;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"><img src="https://lh4.googleusercontent.com/8B5yxCEN2oollZXecK9eKNXuC75Ixe1z_o1_utaFMisiaEIuyeHpB1Ue751mhA10jB1AqcU3Jd7h1E5Pmg5FK6nNggia3w4xNbeeK7X1mISuaf8vdvLuhs4uFHyAHp0_=s1600" style="border:none" height="16px;" width="85px;"></span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><br><span style="font-size:10px;font-family:Verdana;color:#ff9900;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"></span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:10px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"> </span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:12px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline">Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:12px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline">Email</span><span style="font-size:12px;font-family:Verdana;color:#444444;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"> </span><span style="font-size:12px;font-family:Verdana;color:#ff850c;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"><a href="mailto:ahamed@ocedo.com" target="_blank">ahamed@ocedo.com</a></span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:12px;font-family:Verdana;color:#ff850c;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"><a href="mailto:ahamed@ocedo.com" target="_blank"><br></a></span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:9.333333333333332px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline"> </span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt;text-align:justify"><span style="font-size:9.333333333333332px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline">REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER: HRB 717873 </span></p><span style="font-size:9.333333333333332px;font-family:Verdana;color:#4d4f53;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline">MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT</span></div></div></div></div></div></div>
</div>