[patch]setfiles/restorecon applet

Yuichi Nakamura ynakam at hitachisoft.jp
Tue Jul 24 00:08:24 UTC 2007


Hi.

I checked out svn and found many changes.
Thanks for a lot of work.

On Mon, 23 Jul 2007 14:56:57 +0100
"Denis Vlasenko"  wrote:
> On 7/23/07, Bernhard Fischer <rep.dot.nop at gmail.com> wrote:
> > On Mon, Jul 23, 2007 at 10:16:38AM +0900, Yuichi Nakamura wrote:
> > >Hi.
> >
> > in setfiles_full_usage:
> > "\n     -q      Suppress no-error output" \
> > do you mean "Suppress warnings" or "be quiet" or the like?
> >
> > selinux/setfiles.c:
> > Port to BusyBox by 2007
> > s/by 2007/(c) 2007 by/
> 
> I already applied patch to svn...
> 
> > in struct globals, i, personally prefer bools (smaller for me on x86),
> > just as a sidenote.
> 
> bools are smaller than smallints (which are chars on i386)?
> I don't understand how.
> 
> I introduced smallints specifically because *I* want to be able
> to control what they are, not gcc. gcc people hate us.
> (No, not really, but they aren't thinking too much about
> saving on size - see my rant on idiotic recent i386
> stack alignment disaster.)
> 
> > in add_exclude can use use const char*const directory? (didn't look
> > closely). Also, you seem to reimplement something like basename or
> > last_path_component there. Furthermore should return type bool.
> > Initializing len to 0 looks like it is superfluous.
> 
> Yuichi, please look into it and send all follow-on patches relative to svn.
Updated.
- Return bools where it can.
- Updated usage for q option.
- Make -q and -v  exclusive.
- Make add_exclude argument const char *const


> > likewise a couple of others should return a bool:
> > -+static int exclude(const char *file)
> > ++static bool exclude(const char *file)
> >
> > match():
> > +       if (excludeCtr > 0) {
> > +               if (exclude(name)) {
> > +                       goto err;
> > +               }
> > +       }
> > Please change to
> >         if (excludeCtr > 0 && exclude(name))
> >                 goto err;
> > (perhaps in a couple of other places too. (run, from the toplevel
> > sourcedir 'indent selinux/*' for other misc style-cleanups.)
> > near "if (expand_realpath) {", move the
> > +                       if (excludeCtr > 0 && exclude(name))
> > +                               goto err;
> > out of both branches if if-else.
It seems that they are already done by Denis, thanks.

> >
> > restore(): user_only_changed can be bool. Isn't there a sanitize_path()
> > that you could reuse for stripping multiple slashes off?
user_only_changed is now bool.

About sanitize_path, it is pending.
Removing extra slashes is done only there in setfiles.c .
Is removing extra slashes done in other applets?
If so, should it be in libbb?
I've found bb_simplify_path, but it is expanding "." .

> >
> > [snipping the rest of you patch for now]
> > HTH,
> --
> vda

Attached is update.
-- 
Yuichi Nakamura
Hitachi Software Engineering Co., Ltd.
Japan SELinux Users Group(JSELUG): http://www.selinux.gr.jp/
SELinux Policy Editor: http://seedit.sourceforge.net/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: setfiles.5.patch
Type: application/octet-stream
Size: 2056 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20070724/6b64ed6b/attachment-0001.obj 


More information about the busybox mailing list