rc.subr: disabling globbing while processing devfs rules

[Andriy Gapon]

Any objections / concerns for the following change?

An example.
This rule in devfs.rules:
add path da* mode 660 group operator
and this directory:
/data
result in the following rule being actually installed:
100 path data group operator mode 660

Of course, I could refine the pattern in the rule, but I shouldn't have to do
it, because the pattern is for /dev/ entries, not arbitrary files in the
filesystem namespace.

commit 7ce5e9ca5c107e2669f18efa472c1ab14999247c
Author: Andriy Gapon <a...@icyb.net.ua>
Date: Sat Mar 23 10:29:39 2013 +0200

rc.subr: disabling globbing while processing devfs rules in
devfs_rulesets_from_file()

The rules themselves typically have shell-like patterns and it is incorrect
when they get replaced with matching filesystem entries.

Shell magic by: jilles

diff --git a/etc/rc.subr b/etc/rc.subr
index f37ede7..9952c82 100644
--- a/etc/rc.subr
+++ b/etc/rc.subr
@@ -1301,7 +1301,7 @@ make_symlink()
#
devfs_rulesets_from_file()
{
- local file _err _me
+ local file _err _me _opts
file="$1"
_me="devfs_rulesets_from_file"
_err=0
@@ -1314,6 +1314,11 @@ devfs_rulesets_from_file()
debug "$_me: no such file ($file)"
return 0
fi
+
+ # Disable globbing so that the rule patterns are not expanded
+ # by accident with matching filesystem entries.
+ _opts=$-; set -f
+
debug "reading rulesets from file ($file)"
{ while read line
do
@@ -1360,6 +1365,7 @@ devfs_rulesets_from_file()
break
fi
done } < $file
+ case $_opts in *f*) ;; *) set +f ;; esac
return $_err
}


--
Andriy Gapon

[John Baldwin]

Why not use 'local -' instead of the $- magic? That is:

devfs_rulesets_from_file()
{
local file _err _me -

...
set -f
...
}

That would seem to be simpler.

--
John Baldwin
_______________________________________________
freebsd...@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-curre...@freebsd.org"

[Jilles Tjoelker]

On Mon, Apr 01, 2013 at 02:06:50PM -0400, John Baldwin wrote:
> Why not use 'local -' instead of the $- magic? That is:

> devfs_rulesets_from_file()
> {
> local file _err _me -
>
> ...
> set -f
> ...
> }

> That would seem to be simpler.

I had mentioned this possibility on IRC, but this feature is specific to
Almquist-derived shells (ash) and so something more portable was
selected. (It's still not standard because POSIX does not specify
"local" but it works on most shells in use.)

--
Jilles Tjoelker

[John Baldwin]

On Monday, April 01, 2013 3:56:01 pm Jilles Tjoelker wrote:
> On Mon, Apr 01, 2013 at 02:06:50PM -0400, John Baldwin wrote:
> > Why not use 'local -' instead of the $- magic? That is:
>
> > devfs_rulesets_from_file()
> > {
> > local file _err _me -
> >
> > ...
> > set -f
> > ...
> > }
>
> > That would seem to be simpler.
>
> I had mentioned this possibility on IRC, but this feature is specific to
> Almquist-derived shells (ash) and so something more portable was
> selected. (It's still not standard because POSIX does not specify
> "local" but it works on most shells in use.)

rc.subr isn't meant to be portable, it's a script that is part of the FreeBSD
base system. I find the 'local -' syntax more readable (and used the feature
quite a bit in etcupdate).

--
John Baldwin

[Andriy Gapon]

on 01/04/2013 23:16 John Baldwin said the following:

> On Monday, April 01, 2013 3:56:01 pm Jilles Tjoelker wrote:
>> On Mon, Apr 01, 2013 at 02:06:50PM -0400, John Baldwin wrote:
>>> Why not use 'local -' instead of the $- magic? That is:
>>
>>> devfs_rulesets_from_file()
>>> {
>>> local file _err _me -
>>>
>>> ...
>>> set -f
>>> ...
>>> }
>>
>>> That would seem to be simpler.
>>
>> I had mentioned this possibility on IRC, but this feature is specific to
>> Almquist-derived shells (ash) and so something more portable was
>> selected. (It's still not standard because POSIX does not specify
>> "local" but it works on most shells in use.)
>
> rc.subr isn't meant to be portable, it's a script that is part of the FreeBSD
> base system. I find the 'local -' syntax more readable (and used the feature
> quite a bit in etcupdate).
>

You have to set an example in etc/ :-)
I used a script in etc/rc.d as a starting point, Jilles guided me from there.

--
Andriy Gapon