[Bug 1371158] Review Request: ebtree - Elastic binary tree library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1371158



--- Comment #9 from Petr Pisar <ppisar@xxxxxxxxxx> ---
(In reply to Ingvar Hagelund from comment #7)
> > %build
> > +# Some hardening on epel5,6
> > +%if 0%{?rhel} == 5 || 0%{?rhel} == 6
> > +export CFLAGS="%{optflags} -fPIE"
> > +export LDFLAGS="-Wl,-z,relro -z,now"
> > +%endif
> > 
> > TODO: Here you still ignores %{__global_ldflags} for EPEL 5 and 6. The same
> > applies to %check section. The macro is maybe empty there but still for the
> > sake of consistency and those who redefines it locally it should be
> > respected.
> 
> I don't know how often anybody uses %{__global_ldflags} on epel5,6, but OK:
> 
> # Some hardening on epel5,6
> %if 0%{?rhel} == 5 || 0%{?rhel} == 6
> export CFLAGS="%{optflags} -fPIE"
> export LDFLAGS="%{__global_ldflags} -fPIC -DPIC -Wl,-z,relro -z,now"
> %endif
>  
> > TODO: I recommend to add -fPIC -DPIC compiler options when building the
> > shared library object files. They are required for building a shared
> > library. The build passes only because
> > /usr/lib/rpm/redhat/redhat-hardened-cc1 hardening configuration supplies
> > -fPIE.
> 
> Is it enough to just add them to LDFLAGS, like above?
>
-fPIC -DPIC are compiler options. Not linker options. The should be added to
CFLAGS. Not LDFLAGS.


> > TODO: I recommend to build the tests in %build section.
> 
> Why? Isn't this exactly what %check is for?
> 
I think %check is for running tests. Not for compiling code. But it does not
matter much.

> > > TODO: The library SONAME should be two-digit because this not an upstream
> > > versioning. It also does not make sense to start with "6". You could start
> > > with "0.1".
> > (...) 
> > TODO: This is not a 2-digit SONAME. SONAME is what you pass to -soname
> > linker option. 
> 
> Like this?
> 
> +SOREL = 0
> +SOMIN = 1
> 
> (...)
> 
> +libebtree.so: $(OBJS)
> +       $(CC) $(LDFLAGS) -shared -Wl,-soname,libebtree.so.$(SOREL).$(SOMIN)
> -o libebtree.so.$(SOREL).$(SOMIN) $(OBJS)
> +       ln -sf libebtree.so.$(SOREL).$(SOMIN) libebtree.so.$(SOREL)
> +       ln -sf libebtree.so.$(SOREL) libebtree.so
> +
>
No. The SOREL should be "0.1", SOMIN whatever you want (actually you does not
have to use it), and -soname option would be "libebtree.so.$(SOREL)" only.

The idea is to have soname different from whatever upstream could use when he
decided to start using soname.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]