Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Apr 2009 18:12:41 -0400
From:      "Mikhail T." <mi+thun@aldan.algebra.com>
To:        Maksim Yevmenkin <maksim.yevmenkin@gmail.com>
Cc:        "freebsd-bluetooth@freebsd.org" <freebsd-bluetooth@freebsd.org>
Subject:   Re: RFC: obexapp - virtual root folder for each device
Message-ID:  <49E50A59.9000606@aldan.algebra.com>
In-Reply-To: <bb4a86c70904141438y2f8742efxa9c640f21581b2ed@mail.gmail.com>
References:  <bb4a86c70904131640n7cf471c0o7a56a43d722a90e1@mail.gmail.com>	 <49E3DB35.4030601@aldan.algebra.com>	 <bb4a86c70904131909k44aba88dk126d25a4f1fc3744@mail.gmail.com>	 <49E41DBB.5090805@aldan.algebra.com>	 <bb4a86c70904140934h5b967efby4e3238bd97c5ca01@mail.gmail.com>	 <49E4CAE1.6090506@aldan.algebra.com>	 <bb4a86c70904141155h205b78caq95d5f2d30a583376@mail.gmail.com>	 <49E4EA14.80300@aldan.algebra.com> <bb4a86c70904141438y2f8742efxa9c640f21581b2ed@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Maksim Yevmenkin ΞΑΠΙΣΑΧ(ΜΑ):
> hopefully the latest patch will work for everyone.
>   
Little nits this time...

1. Even when the service is not running as root to begin with (and thus
chroot is impossible), a user owning multiple devices may wish to have
separate directories for each one. Yet, the new -R switch, as proposed,
requires successful chroot into a matching entry... It does not need to.
How about:

    +		case 'R': /* virtualize root for each device */
    +			context.vroot = 1;
    +			if (getuid() == 0)
    +				context.secure = 1;
    +			break;
    +

chdir should be used instead of chroot in this case (-R was given, and a
matching entry is found, but we aren't running as root).

2. When going through the list of possible subdirectories-candidates,
hardcoding the number 3 is a bit dangerous. Using either

    sizeof(root)/sizeof(root[0])

or simply going, until hitting NULL:

    char const *root[] = { NULL, NULL, NULL, NULL }, **r;
    ...
    foreach (r -> root; *r; r++) {

        log_debug("%s(): Checking for %s/%s subdirectory",
            __func__, context->root, *r);
        ...

would be a bit safer going forward -- what if some other parameter (such
as an environment variable) may some day be added to the list of
considerations?

3. After starting up, with the -R (or the -r) option, should/does not
the daemon chdir into the specified top-level directory? And if so,
there is no need to assemble the context->root with strlcat -- just
perform chdir into the relative root[n] (or *r in my example). The
chroot can then happen to ".". After chdir-ing, you can populate the
actual contet->root with getcwd(context->root, PATH_MAX) -- a faster
equivalent to using realpath(3).

This is not material, but the fewer cases, where a hard-coded PATH_MAX
is used instead of the POSIX-approved pathconf(2), the better...

Yours,

    -mi

P.S. Thank you very much for keeping the wallabies in the manual's
examples. Better creatures are not to be found, and it is certainly
nicer to use them instead of the ubiquitous "foo" and "bar".



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49E50A59.9000606>