Re: [linux-lvm] [lvm-devel] MUSL fun

Re: [linux-lvm] [lvm-devel] MUSL fun

[Zdenek Kabelac]

Dne 13.2.2016 v 20:22 Brendan Heading napsal(a):
> Bernd,
>
> [cc buildroot]
>
> Like you I'd been investigating this issue from the buildroot end.
> Since October I've had no spare time to look at buildroot fixes.
>
> I was hoping for an explanation about why LVM implemented the above
> behaviour, but I never got one, and I didn't fancy working on a patch
> that would never get accepted when other projects were more open ..
>
> I think the best approach is probably just to add the freopen() patch
> as Romain suggested and keep the patch available in buildroot in the
> hope that we can build support for LVM to accept it.
>
> Brendan
>
>
> On 13 February 2016 at 18:48, Bernd Kuhls <bernd.kuhls-zqRNUXuvxA0b1SvskN2V4Q@public.gmane.org> wrote:
>> Am Tue, 15 Sep 2015 17:43:58 +0100 schrieb Brendan Heading:
>>
>>> In theory a straightforward fix would be to do something like:
>>>
>>> if (!freopen(NULL, "r", stdin))
>>>       goto out;
>>>
>>> .. but I suspect there is a good reason why this isn't done. Can anyone
>>> shed any light on this ? If not I will submit a patch.
>>
>> Hi Brendan,
>>
>> the buildroot project also supports musl, the build of lvm2 fails due to
>> the problem you described. Did you find a working solution for the
>> problem? A patch I submitted to the buildroot project, based on patches I
>> found at the Alpinelinux project, raised some questions I am unable to
>> answer: http://patchwork.ozlabs.org/patch/573519/
>>
>> Regards, Bernd


Let's start from the beginning -

-- 
commands/toolcontext.c: In function 'create_toolcontext':
commands/toolcontext.c:1796:10: error: assignment of read-only variable 'stdin'
     stdin = new_stream;
-- 

So now - from where comes the idea  of  'stdin' being read-only variable ?
Looking at some POSIX spec e.g.:
http://pubs.opengroup.org/onlinepubs/009695399/functions/stdin.html

there is no sign of having this read-only var.

So what kind of OS this actually is ?


There is a reason from lvm2 implementation here - freopen() is not giving 
application full control over internal buffer allocation and we need to be 
sure we lock-in memory for critical section - and some glibc versions are 
allocating buffers here via 'mmap' call.

That said - there could be accepted a patch checking in configure  for
'read-only' stdin  - and using then #ifdef compilation that would
replace use of  internal lvm2 reopen code with libc function.

But the currently posted patch may possibly cause some other regressions.

Regards

Zdenek

Re: [linux-lvm] [lvm-devel] MUSL fun

[Brendan Heading]

> commands/toolcontext.c: In function 'create_toolcontext':
> commands/toolcontext.c:1796:10: error: assignment of read-only variable
> 'stdin'
>     stdin = new_stream;
> --
>
> So now - from where comes the idea  of  'stdin' being read-only variable ?
> Looking at some POSIX spec e.g.:
> http://pubs.opengroup.org/onlinepubs/009695399/functions/stdin.html
>
> there is no sign of having this read-only var.
>
> So what kind of OS this actually is ?

musl's rather conservative interpretation appears to be that since the
values of stdin, stdout and stderr are explicitly defined by POSIX, it
is implicit that the variables holding these values should be const.

http://www.openwall.com/lists/musl/2013/03/18/1

There's not a lot of point in debating the rights or wrongs of this.
If it were me, I'd take a pragmatic approach, and if it were me I'd
make this a configurable option, but this defeats musl's stated
objectives and they won't accept a patch to change it.

> There is a reason from lvm2 implementation here - freopen() is not giving
> application full control over internal buffer allocation and we need to be
> sure we lock-in memory for critical section - and some glibc versions are
> allocating buffers here via 'mmap' call.
>
> That said - there could be accepted a patch checking in configure  for
> 'read-only' stdin  - and using then #ifdef compilation that would
> replace use of  internal lvm2 reopen code with libc function.

I had a suspicion you would have a good reason.

On first glance your proposal sounds like a good compromise and one
that would be worth exploring.

Brendan

Re: [linux-lvm] [lvm-devel] MUSL fun

[Brendan Heading]

>> There is a reason from lvm2 implementation here - freopen() is not giving
>> application full control over internal buffer allocation and we need to be
>> sure we lock-in memory for critical section - and some glibc versions are
>> allocating buffers here via 'mmap' call.
>>
>> That said - there could be accepted a patch checking in configure  for
>> 'read-only' stdin  - and using then #ifdef compilation that would
>> replace use of  internal lvm2 reopen code with libc function.
>
> I had a suspicion you would have a good reason.
>
> On first glance your proposal sounds like a good compromise and one
> that would be worth exploring.

As an alternative idea, what about changing the codebase not to use
stdin/stdout/stderr, using it's own file descriptor names and calling
dup2() as appropriate to set them up on startup ?

Just flying the kite - I haven't looked to see how painful this might be.

Brendan

Re: [linux-lvm] [lvm-devel] MUSL fun

[Zdenek Kabelac]

Dne 13.2.2016 v 22:31 Brendan Heading napsal(a):
>>> There is a reason from lvm2 implementation here - freopen() is not giving
>>> application full control over internal buffer allocation and we need to be
>>> sure we lock-in memory for critical section - and some glibc versions are
>>> allocating buffers here via 'mmap' call.
>>>
>>> That said - there could be accepted a patch checking in configure  for
>>> 'read-only' stdin  - and using then #ifdef compilation that would
>>> replace use of  internal lvm2 reopen code with libc function.
>>
>> I had a suspicion you would have a good reason.
>>
>> On first glance your proposal sounds like a good compromise and one
>> that would be worth exploring.
>
> As an alternative idea, what about changing the codebase not to use
> stdin/stdout/stderr, using it's own file descriptor names and calling
> dup2() as appropriate to set them up on startup ?
>
> Just flying the kite - I haven't looked to see how painful this might be.

Hi

Well don't know anyone who would play with this approach which is several 
orders in magnitude more difficult then the plain and simple #ifdef.

I also assume we may probably even overwrite value of stdin via tricky pointer 
magic so compiler will not be able to spot mods of stdin in buildtime.

Regards

Zdenek