On 08/22/2017 02:28 PM, Nadav Har'El wrote:
On Tue, Aug 22, 2017 at 2:48 PM, Justin Cinkelj
<[email protected] <mailto:[email protected]>> wrote:
> What does it not fix?
The $892 mentions environ varibales with spaces or empty one. And
command line parsing is done in loader.cc to get loader options,
and in commands.cc, to get argv/argc for app, and to implement
runscript. This one only touched loader.cc options, and ensured
that commands.cc can get argv[i] with spaces. It didn't fix
runcript. Also, when commands.cc re-assembles and re-parses
cmdline, the initial "aa bb" argv[i] get incorrectly split on space.
> everything else in the command line is glued together again into
one string
In
https://github.com/cloudius-systems/osv/blob/master/loader.cc#L319
<https://github.com/cloudius-systems/osv/blob/master/loader.cc#L319>
// concatenate everything
for (auto i = 0; i < ac; i++) {
line += std::string(av[i]) + " ";
}
It doesn't expect av[i] to contain space. Also " or ' or \ inside
would not work, I guess.
> above parse_cmdline() there is an old (3 year old) comment saying it
cannot use std::string.
> I guess you checked and this is incorrect?
I have not checked. I just blindly tried to use some existing
parser, and not invent/write my own.
So this code might be broken and problematic.
PS:
I also used not only debug_early, but also printf, and even
std::cout, and it worked - for me.
On the other side, Gregor complained in
https://github.com/cloudius-systems/osv/issues/892#issuecomment-323910628
<https://github.com/cloudius-systems/osv/issues/892#issuecomment-323910628>
about
std::cout not working (inside parse_cmdline, I think). So it might
depend on gcc/boost/something version.
> Why do we even need this argc/argv? We don't, really....
And neither we don't really need this patch ? ;) I vote to abandon it.
I was looking if there is way to get from boost::tokenizer both
token and its start/end position in original string.
Then, loader would consume first few tokes (the loader options,
they all start with '-') from argv array.
The remaining part would be consumed by commands.cc - end position
of last loader option would be passed as commandline to it.
--env=AAA="aa bb"
is problematic case here, as it contains spaces.
So we need to support quoting and spaces and escaping.
Nothing from C++/std/boost can be used.
Why, because it is done early?
Or because you think nothing from C++/std/boost fits?
Things from std/boost would fit.
My problem/worry is it is done early, and I'm afraid that Gregor could
not use std:cout because of that.
@Gergor
On what platform did you try to use std:cout in parse_cmdline (loader.cc)?
Was enough to add a single "cout << "blah" << endl;" to get well
repeatable problems?
I would like to be able to check for that problem, but fedora 25 as
build system seems to produce immune image.
strtok from C doesn't support quoting/escaping.
Are you aware of any C alternative to strtok with quoting/escaping
support? Just asking before trying to do it myself.
stresep would fit (I think), it has escape character. It is not in nm
loader.elf output, but if I can find BSD licensed file (and it seems to
be part of netbsd), then I will give it a try.
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
