Re: small fix of libtool.m4

2016-05-09 Thread Christian
Hey Richard,

sorry i didn’t see your patch. as i wrote in the beginning, this is one of my 
first patches i committed and i might have done something wrong. In this case i 
haven’t had a look at the bug list first. sorry for that.

i don’t want to say your patch is not working, since i didn’t try it yet, but 
i’ll give it a try tomorrow and tell you if it fixed the problem. right now it 
looks like a nice solution to me. and it does fix the problem at it’s root 
source! but as is said, i’ll give it a try tomorrow and write back again.

kind regards
christian

> On May 9, 2016, at 9:55 PM, Richard PALO  wrote:
> 
> Le 10/05/16 02:43, Christian a écrit :
>> i’ve had a look again at libtool.m4 and don’t really get why RM is set 
>> wrong. obviously the _LT_CONFIG macro literally requires 
>> _LT_FILEUTILS_DEFAULT, which should set RM to ‘rm -f’. I also found several 
>> uses of $RM with different options, sometimes even ‘-f’. So actually i am 
>> not sure what would be the best solution here.
>> 
>> right now i could change the patch like proposed by Eric Blake to the one i 
>> attached. It will at least fix the direct problem i encountered. But maybe 
>> for the long term, someone should check and maintain the whole libtool.m4 
>> file?!
>> 
>> kind regards
>> christian
> 
> I filed https://savannah.gnu.org/support/index.php?108987 a few months ago, 
> are you saying the proposed patches don't fix this?
> 
> -- 
> Richard PALO
> 




Re: small fix of libtool.m4

2016-05-09 Thread Richard PALO
Le 10/05/16 02:43, Christian a écrit :
> i’ve had a look again at libtool.m4 and don’t really get why RM is set wrong. 
> obviously the _LT_CONFIG macro literally requires _LT_FILEUTILS_DEFAULT, 
> which should set RM to ‘rm -f’. I also found several uses of $RM with 
> different options, sometimes even ‘-f’. So actually i am not sure what would 
> be the best solution here.
> 
> right now i could change the patch like proposed by Eric Blake to the one i 
> attached. It will at least fix the direct problem i encountered. But maybe 
> for the long term, someone should check and maintain the whole libtool.m4 
> file?!
> 
> kind regards
> christian

I filed https://savannah.gnu.org/support/index.php?108987 a few months ago, 
are you saying the proposed patches don't fix this?

-- 
Richard PALO




Re: small fix of libtool.m4

2016-05-09 Thread Christian
i’ve had a look again at libtool.m4 and don’t really get why RM is set wrong. 
obviously the _LT_CONFIG macro literally requires _LT_FILEUTILS_DEFAULT, which 
should set RM to ‘rm -f’. I also found several uses of $RM with different 
options, sometimes even ‘-f’. So actually i am not sure what would be the best 
solution here.

right now i could change the patch like proposed by Eric Blake to the one i 
attached. It will at least fix the direct problem i encountered. But maybe for 
the long term, someone should check and maintain the whole libtool.m4 file?!

kind regards
christian



cfgfile.patch
Description: Binary data



> On May 9, 2016, at 1:59 PM, Roumen Petrov  wrote:
> 
> Christian wrote:
>> so today i gave it a shot again and put a debug output right before the ‘$RM 
>> “$cfgfile”’. For some reason RM is set to ‘/bin/rm’ only. no ‘-f’. i’ll try 
>> to figure out where that might come from.
> Perhaps build package is libxslt .
> Issue is already reported many times. Project use AC_PATH_PROG to find rm 
> command.
> 
> May be change variable name to lt_RM is solution
> 
> Regards,
> Roumen
> 



Re: small fix of libtool.m4

2016-05-09 Thread Roumen Petrov

Christian wrote:

so today i gave it a shot again and put a debug output right before the ‘$RM 
“$cfgfile”’. For some reason RM is set to ‘/bin/rm’ only. no ‘-f’. i’ll try to 
figure out where that might come from.

Perhaps build package is libxslt .
Issue is already reported many times. Project use AC_PATH_PROG to find 
rm command.


May be change variable name to lt_RM is solution

Regards,
Roumen




Re: small fix of libtool.m4

2016-05-09 Thread Christian
so today i gave it a shot again and put a debug output right before the ‘$RM 
“$cfgfile”’. For some reason RM is set to ‘/bin/rm’ only. no ‘-f’. i’ll try to 
figure out where that might come from.
anyway, thx for the hint :)

> On May 8, 2016, at 8:57 PM, Christian  wrote:
> 
> thx Eric for taking a look at my patch.
> 
> it seems to be a good idea to use ‘rm -f’ and i also think it looks like $RM 
> should already be set to ‘rm -f’. but i don’t get why it should fail than.
> what i’ll do next is check if $RM really is set to ‘rm -f’ using a  debug 
> print. couldn’t do it right now, just in case it might be set different for 
> some reason.
> 
>> On May 6, 2016, at 7:48 AM, Eric Blake  wrote:
>> 
>> On 05/05/2016 11:22 PM, Christian wrote:
>>> So i found that if you’re running ‘./configure’ on a project that depends 
>>> on 
>>> libtool, it might happen that the script will end up with the following 
>>> error: 
>>> “/bin/rm: cannot remove 'libtoolT': No such file or directory”. I did some 
>> 
>>> 
>>>cfgfile=${ofile}T
>>>trap "$RM \"$cfgfile\"; exit 1" 1 2 15
>>> -$RM "$cfgfile"
>>> +if test -e "$cfgfile" ; then
>>> +  $RM "$cfgfile"
>>> +fi
>> 
>> That's a TOCTTOU data race.  Wouldn't it be better to just use 'rm -f'?
>> In fact, isn't $RM supposed to be including -f automatically?
>> 
>> -- 
>> Eric Blake   eblake redhat com+1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>> 
> 




Re: small fix of libtool.m4

2016-05-08 Thread Christian
thx Eric for taking a look at my patch.

it seems to be a good idea to use ‘rm -f’ and i also think it looks like $RM 
should already be set to ‘rm -f’. but i don’t get why it should fail than.
what i’ll do next is check if $RM really is set to ‘rm -f’ using a  debug 
print. couldn’t do it right now, just in case it might be set different for 
some reason.

> On May 6, 2016, at 7:48 AM, Eric Blake  wrote:
> 
> On 05/05/2016 11:22 PM, Christian wrote:
>> So i found that if you’re running ‘./configure’ on a project that depends on 
>> libtool, it might happen that the script will end up with the following 
>> error: 
>> “/bin/rm: cannot remove 'libtoolT': No such file or directory”. I did some 
> 
>> 
>> cfgfile=${ofile}T
>> trap "$RM \"$cfgfile\"; exit 1" 1 2 15
>> -$RM "$cfgfile"
>> +if test -e "$cfgfile" ; then
>> +  $RM "$cfgfile"
>> +fi
> 
> That's a TOCTTOU data race.  Wouldn't it be better to just use 'rm -f'?
> In fact, isn't $RM supposed to be including -f automatically?
> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 




Re: small fix of libtool.m4

2016-05-06 Thread Eric Blake
On 05/05/2016 11:22 PM, Christian wrote:
> So i found that if you’re running ‘./configure’ on a project that depends on 
> libtool, it might happen that the script will end up with the following 
> error: 
> “/bin/rm: cannot remove 'libtoolT': No such file or directory”. I did some 

>  
>  cfgfile=${ofile}T
>  trap "$RM \"$cfgfile\"; exit 1" 1 2 15
> -$RM "$cfgfile"
> +if test -e "$cfgfile" ; then
> +  $RM "$cfgfile"
> +fi

That's a TOCTTOU data race.  Wouldn't it be better to just use 'rm -f'?
 In fact, isn't $RM supposed to be including -f automatically?

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature