On Tue, Aug 20, 2013 at 01:23:18PM -0600, Eric Blake wrote: > On 08/20/2013 08:39 AM, Guido Günther wrote: > > by adding it to AM_LDFLAGS instead of every linking rule and > > by avoiding a forked grep. > > --- > > Daniel kind of nacked the AM_LDFLAGS part already but I think it's a > > reasonable cleanup. We should rather use AM_LDFLAGS everywhere which > > (we currently don't and which would be another cleanup). Or are there > > any reasons to not have a read only GOT? > > Personally, I like the idea of using AM_LDFLAGS everywhere, but as > Daniel and I have opposing opinions, we'll need a third person to speak up. O.k. I think we lose more by not having eqaul AM_LDFLAGS everywhere since there are already several places that did lack some flags since it's error prone to maintain all locations correctly. > > > +++ b/m4/virt-linker-relro.m4 > > @@ -22,10 +22,10 @@ AC_DEFUN([LIBVIRT_LINKER_RELRO],[ > > AC_MSG_CHECKING([for how to force completely read-only GOT table]) > > > > RELRO_LDFLAGS= > > - `$LD --help 2>&1 | grep -- "-z relro" >/dev/null` && \ > > - RELRO_LDFLAGS="-Wl,-z -Wl,relro" > > - `$LD --help 2>&1 | grep -- "-z now" >/dev/null` && \ > > - RELRO_LDFLAGS="$RELRO_LDFLAGS -Wl,-z -Wl,now" > > + case `$LD --help 2>&1` in > > + *"-z relro"*) RELRO_LDFLAGS="-Wl,-z -Wl,relro" ;& > > ';&' is a bashism; it is not portable to configure scripts > > > + *"-z now"*) RELRO_LDFLAGS="$RELRO_LDFLAGS -Wl,-z -Wl,now" ;; > > + esac > > Instead, this needs to be: > > ld_help=`$LD --help 2>&1` > case $ld_help in > *"-z relro"*) RELRO_LDFLAGS="-Wl,-z -Wl,relro" ;; > esac > case $ld_help in > *"-z now"*) RELRO_LDFLAGS="$RELRO_LDFLAGS -Wl,-z -Wl,now" ;; > esac > > > > @@ -1812,7 +1814,6 @@ libvirt_la_LDFLAGS = \ > > -version-info $(LIBVIRT_VERSION_INFO) \ > > $(LIBVIRT_NODELETE) \ > > $(AM_LDFLAGS) \ > > - $(RELRO_LDFLAGS) \ > > $(CYGWIN_EXTRA_LDFLAGS) \ > > $(MINGW_EXTRA_LDFLAGS) \ > > $(NULL) > > Hmm - maybe I see why Daniel expressed his opinion for not using > $(AM_LDFLAGS) - notice that libraries to NOT use $(PIE_LDFLAGS)... > > > @@ -1964,7 +1963,6 @@ virtlockd_CFLAGS = \ > > virtlockd_LDFLAGS = \ > > $(AM_LDFLAGS) \ > > $(PIE_LDFLAGS) \ > > - $(RELRO_LDFLAGS) \ > > $(CYGWIN_EXTRA_LDFLAGS) \ > > $(MINGW_EXTRA_LDFLAGS) \ > > $(NULL) > > ...but executables do. But even then, maybe we want: > > LIBRARY_LDFLAGS = $(AM_LDFLAGS) > EXEC_LDFLAGS = $(AM_LDFLAGS) $(PIE_LDFLAGS) That would be fine with me but I'd do this as a separate cleanup since AM_LDFLAGS currently only carries flags suitable for both lib and executable builds. Cheers, -- Guido > then use the appropriate $({LIBRARY,EXEC}_LDFLAGS) in each place, rather > than having lots of duplicate $(PIE_LDFLAGS)? > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list