| Message ID | 20180626102521.28129-1-emil.l.velikov@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 26, 2018 at 12:27 PM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > From: Emil Velikov <emil.velikov@collabora.com> > > Currently each profile is handles in two stages: > - a unique value is returned for a set of patterns matches > - depending on the value the profile/essid is added to global lists > > A shorter and simpler solution is to omit the unnecessary value > passing/processing all together. > > Cc: Jouke Witteveen <j.witteveen@gmail.com> > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > --- > Jouke, let me know if you'd like this rebased on top of your patch. > Pardon if you're getting this twice. The initial submission was rejected > by the ML :-\ > --- > src/wifi-menu | 40 ++++++++++++++-------------------------- > 1 file changed, 14 insertions(+), 26 deletions(-) > > diff --git a/src/wifi-menu b/src/wifi-menu > index 42c4c53..928cf5e 100755 > --- a/src/wifi-menu > +++ b/src/wifi-menu > @@ -29,34 +29,22 @@ quote_safe() { > fi > } > > -# Fill PROFILES and ESSIDS with the profile names and essids of the profiles > -# for interface $1 > +# Fill GENERATED, PROFILES and ESSIDS with the profile names and essids of the Ah, right. I'll add "GENERATED". > +# profiles for interface $1 > init_profiles() { > - local i=0 essid profile > + local i=0 profile > while IFS= read -r profile; do > - essid=$( > - unset Interface ESSID > - source "$PROFILE_DIR/$profile" > /dev/null > - if [[ "$Interface" = "$1" && -n "$ESSID" ]]; then > - printf "%s" "$ESSID" > - if [[ "$Description" =~ "Automatically generated" ]]; then > - return 2 > - else > - return 1 > - fi > - fi > - return 0 > - ) > - case $? in > - 2) > - GENERATED+=("$profile") > - ;& > - 1) > - PROFILES[i]=$profile > - ESSIDS[i]=$essid > - (( ++i )) > - ;; > - esac Okay, I'll add a comment explaining that we do things this way for sandboxing reasons. We want to source the profile in a subshell to contain any side-effects. Thanks for your contribution! - Jouke
On 26 June 2018 at 11:40, Jouke Witteveen <j.witteveen@gmail.com> wrote: > On Tue, Jun 26, 2018 at 12:27 PM Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >> From: Emil Velikov <emil.velikov@collabora.com> >> >> Currently each profile is handles in two stages: >> - a unique value is returned for a set of patterns matches >> - depending on the value the profile/essid is added to global lists >> >> A shorter and simpler solution is to omit the unnecessary value >> passing/processing all together. >> >> Cc: Jouke Witteveen <j.witteveen@gmail.com> >> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> >> --- >> Jouke, let me know if you'd like this rebased on top of your patch. >> Pardon if you're getting this twice. The initial submission was rejected >> by the ML :-\ >> --- >> src/wifi-menu | 40 ++++++++++++++-------------------------- >> 1 file changed, 14 insertions(+), 26 deletions(-) >> >> diff --git a/src/wifi-menu b/src/wifi-menu >> index 42c4c53..928cf5e 100755 >> --- a/src/wifi-menu >> +++ b/src/wifi-menu >> @@ -29,34 +29,22 @@ quote_safe() { >> fi >> } >> >> -# Fill PROFILES and ESSIDS with the profile names and essids of the profiles >> -# for interface $1 >> +# Fill GENERATED, PROFILES and ESSIDS with the profile names and essids of the > > Ah, right. I'll add "GENERATED". > >> +# profiles for interface $1 >> init_profiles() { >> - local i=0 essid profile >> + local i=0 profile >> while IFS= read -r profile; do >> - essid=$( >> - unset Interface ESSID >> - source "$PROFILE_DIR/$profile" > /dev/null >> - if [[ "$Interface" = "$1" && -n "$ESSID" ]]; then >> - printf "%s" "$ESSID" >> - if [[ "$Description" =~ "Automatically generated" ]]; then >> - return 2 >> - else >> - return 1 >> - fi >> - fi >> - return 0 >> - ) >> - case $? in >> - 2) >> - GENERATED+=("$profile") >> - ;& >> - 1) >> - PROFILES[i]=$profile >> - ESSIDS[i]=$essid >> - (( ++i )) >> - ;; >> - esac > > Okay, I'll add a comment explaining that we do things this way for > sandboxing reasons. We want to source the profile in a subshell to > contain any side-effects. > Hmm, I did not think of that one. Thanks for the prompt explanation. -Emil
diff --git a/src/wifi-menu b/src/wifi-menu index 42c4c53..928cf5e 100755 --- a/src/wifi-menu +++ b/src/wifi-menu @@ -29,34 +29,22 @@ quote_safe() { fi } -# Fill PROFILES and ESSIDS with the profile names and essids of the profiles -# for interface $1 +# Fill GENERATED, PROFILES and ESSIDS with the profile names and essids of the +# profiles for interface $1 init_profiles() { - local i=0 essid profile + local i=0 profile while IFS= read -r profile; do - essid=$( - unset Interface ESSID - source "$PROFILE_DIR/$profile" > /dev/null - if [[ "$Interface" = "$1" && -n "$ESSID" ]]; then - printf "%s" "$ESSID" - if [[ "$Description" =~ "Automatically generated" ]]; then - return 2 - else - return 1 - fi - fi - return 0 - ) - case $? in - 2) - GENERATED+=("$profile") - ;& - 1) - PROFILES[i]=$profile - ESSIDS[i]=$essid - (( ++i )) - ;; - esac + unset Interface ESSID + source "$PROFILE_DIR/$profile" > /dev/null + if [[ "$Interface" != "$1" || -z "$ESSID" ]]; then + continue; + fi + if [[ "$Description" =~ "Automatically generated" ]]; then + GENERATED+=("$profile") + fi + PROFILES[i]=$profile + ESSIDS[i]=$ESSID + (( ++i )) done < <(list_profiles) }