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

Re: [lvm-devel] [RFC][PATCH 1/5] support command string with space



Takahiro Yasui <tyasui redhat com> writes:

> On 09/30/09 16:24, Petr Rockai wrote:
>> IIUIC, the code will replace "\ " with "  " (double space) -- which may or may
>> not be correct, depending on where the space appears. It also breaks any code
>> that may have previously expected "\ " to have no special meaning, even though I
>> doubt any exists.
>
> Yes, this approach replace "\ " with "  ", and it will work correctly unless
> "\ " is used for other purpose previously. Current lvm codes use lvm_split()
> only in early stage to split options and there is no case you concerns.
There may not be *right now* -- I still think this is a bug waiting to
happen. Shell scripts are full of escaping bugs and in dmeventd we have a
simple way around (see below). One example for all -- I know it may be sick,
but you can have device node paths with spaces in them, and I expect this will
break -- either you forget to escape them, or the un-escaping produces wrong
paths. (And as surprising as it may sound, spaces in device names work with
current code -- I can create PVs and VGs on them and I can filter them out or
in.)

>> Anyway, a more correct solution would entail a lvm2_runv (or similarly named)
>> function, that would take char **argv -- no escaping issues to care
>> about. Since this just adds to the library, it should be backwards compatible
>> just fine.
>
> I can add an interface like lvm2_runv. But there is another approach using
> single-quotes around an option with spaces as we do in a shell (e.g. bash).
And how you quote single-quotes then? Doesn't work. Please use something
reliable and robust -- like passing argv, or using varargs function (like
execl). This eliminates all quoting bugs quite simply.

Yours,
   Petr.


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