Re: small fix of libtool.m4
Hey together, today i gave it a try with Richards patch and that one works. How can we ‘push’ that patch? As he wrote, it’s on savannah since the end of February. 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
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
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
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
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
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
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
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
small fix of libtool.m4
Hi there,this is the first patch i am committing and i hope i’ll do it right, but please be patient with me if something went wrong accidentally.DESCRIPTION: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 research and found the place within libtool.m4 which tries to remove that file and added a ’test -e’ before the call to ‘rm’. that way the file will get removed if it’s there, but no error will occur if not.CHANGELOG:* m4/libtool.m4 safely try to remove temporary cfgfileplease find the patch attached to this email. I hope i provided everything that’s necessary to get this one fixed. if i did something wrong or you need some information from me, please contact me.kind regardschristian cfgfile.patch Description: Binary data