All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takahiro Yasui <tyasui@redhat.com>
To: lvm-devel@redhat.com
Subject: [RFC][PATCH 1/5] support command string with space
Date: Fri, 16 Oct 2009 15:00:53 -0400	[thread overview]
Message-ID: <4AD8C2E5.6020105@redhat.com> (raw)
In-Reply-To: <87zl8bd5h4.fsf@twilight.int.mornfall.net.>

On 10/01/09 01:54, Petr Rockai wrote:
> 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.

I see. Your approach, lvm2_runv, seems quite safe, and I will implement it.
Thank you for the helpful comments.

Thanks,
Taka



  reply	other threads:[~2009-10-16 19:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-30  0:28 [RFC][PATCH 1/5] support command string with space Takahiro Yasui
2009-09-30 20:24 ` Petr Rockai
2009-09-30 21:19   ` Takahiro Yasui
2009-10-01  5:54     ` Petr Rockai
2009-10-16 19:00       ` Takahiro Yasui [this message]
2009-10-06 11:47   ` Alasdair G Kergon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AD8C2E5.6020105@redhat.com \
    --to=tyasui@redhat.com \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.