[oe] [meta-oe][PATCH v2] systemd: Replace use of %m in printf formats

[Phil Blundell]

On Tue, 2011-07-05 at 14:14 -0700, Khem Raj wrote:
> On Mon, Jul 4, 2011 at 2:39 AM, Phil Blundell <philb at gnu.org> wrote:
> > On Sat, 2011-07-02 at 17:47 -0700, Khem Raj wrote:
> >> +-        if ((fd = mkostemp(temp, O_CLOEXEC|O_CREAT|O_WRONLY)) < 0) {
> >> ++        if ((fd = mkstemp(temp)) < 0) {
> >
> > This change looks like it will effectively remove O_CLOEXEC without
> > reinstating it anywhere else.  Have you verified this is a safe thing to
> > do?
> 
> Initial tests on uclibc seems to work well. but this needs more testing
> thats why the patch is only applied for uclibc based systems

Just to reiterate what I mentioned on IRC the other day, I don't think
empirical testing is the right way to address this problem, at least not
in isolation.  Absent an exceptionally comprehensive testsuite, there is
too much risk that some kind of failure would be overlooked.  I think
code inspection is required here to establish whether O_CLOEXEC is
needed and what to do about it.  The easy answer to the latter is
probably just to add an appropriate fcntl() call.

> >> ++            char dev[20];
> >> +                 int prio = 0, k;
> >> +
> >> +                 if ((k = fscanf(m->proc_swaps,
> >> +-                                "%ms "  /* device/file */
> >> ++                                "%19s "  /* device/file */
> >> +                                 "%*s "  /* type of swap */
> >
> > Is 19 characters definitely enough?  If this is potentially a pathname
> > then it could be much longer than that.
> >
> 
> it just reads /dev/* so should be ok.

I haven't checked the actual code, but the implication from the variable
names and comments shown above is that it's reading /proc/swaps.  If
that's the case then, in the situation where you're swapping to a file,
the pathnames involved could be arbitrarily long.

p.