Re: [PATCH] Cleanup of the quick dirty fix from last week

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/09/2012 08:00 AM, Martin Kletzander wrote:
> I tried lots of different solutions and this seems like the most clean
> and readable one.
> ---
>  src/lxc/lxc_container.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 

> @@ -511,10 +509,14 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot)
>           * tmpfs is limited to 64kb, since we only have device nodes in there
>           * and don't want to DOS the entire OS RAM usage
>           */
> +#if HAVE_SELINUX
>          if (virAsprintf(&opts, "mode=755,size=65536%s%s%s",
>                          con ? ",context=\"" : "",
>                          con ? (const char *)con : "",
>                          con ? "\"" : "") < 0) {
> +#else
> +        if (virAsprintf(&opts, "mode=755,size=65536") < 0) {
> +#endif

I'm not a big fan of unbalanced '{' (two open and one close, because
only the open was protected by ifdef), if only because editors that are
aware of indentation but not of #ifdef will mis-indent code that
follows.  I'm thinking this might read better, and avoid the unbalanced {}:

#if HAVE_SELINUX
    security_context_t con;
#endif

    VIR_DEBUG("Mounting basic filesystems %s pivotRoot=%d",
NULLSTR(srcprefix), pivotRoot);
...

#if HAVE_SELINUX
    if (con)
        ignore_value(virAsprintf(&opts,
                "mode=755,size=65536,context=\"%s\"",
                          (const char *)con));
    else
#endif
        opts = strdup("mode=755,size=65536);
    if (!opts) {
        virReportOOMError();
        goto cleanup;
    }

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]