ver6(Re: [PATCH 2/8] busybox -- SELinux option support for coreutils: ver3

[Yuichi Nakamura]

On Thu, 8 Mar 2007 13:31:42 +0100
Bernhard Fischer  wrote:
> Could you please use diff --show-c-function, makes a review easier, TIA.
OK, I will use it.

> In a couple of places there is still a space missing; E.g.
> +               if(!is_selinux_enabled()) {
> et al.
Fixed.

> Index: coreutils/install.c
> ===================================================================
> --- coreutils/install.c (revision 17961)
> +++ coreutils/install.c (working copy)
> ...
> +#if ENABLE_SELINUX
> +static int use_default_selinux_context = 1;
> 
> smallint / bool smaller?
using bool.

> In
> +static void setdefaultfilecon(const char *path) {
> There are several
> +       if (whatever) {
> +               freecon(scontext);
> +               return;
> +       }
> 
> Smaller to goto one single freecon(scontext) ? (i.e. near strcmp just
> goto outtahere)
Using goto now.

> Your error strings in e.g. install_main are too verbose for my taste,
> would it be possible to chop them a bit?
Removed some error string by 
replacing warning message with selinux_or_die.

> +#if ENABLE_SELINUX
> +               if (((flags & FILEUTILS_PRESERVE_SECURITY_CONTEXT)
> +                || (flags & FILEUTILS_SET_SECURITY_CONTEXT))
> +                && is_selinux_enabled() > 0) {
> +                       security_context_t con;
> +                       if(getfscreatecon(&con) == -1) {
> 
> missing space
Fixed

> 
> +                               bb_perror_msg ("cannot getfscreatecon");
> 
> s/cannot //
> 
> +                               return -1;
> +                       }
> same for setfilecon
Fixed.

Yuichi Nakamura



-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox-coreutils-copy-02.v6.patch
Type: application/octet-stream
Size: 7875 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20070309/3cce9566/attachment-0001.obj