Skip to content

Commit

Permalink
Avoid unnecessary zlib DT_NEEDED entries
Browse files Browse the repository at this point in the history
31fc193 removed multiple superfluous
DT_NEEDED entries, but left zlib alone. At the time, I only paid
attention to libraries that were absent from `DT_NEEDED` when
`-Wl,--as-needed` was passed to CPPFLAGS and zlib was not one of them
(as libuutil linked to it). A `DT_NEEDED` entry on zlib from libuutil
makes absolutely no sense, but I had assumed that the toolchain would be
smart enough to catch all cases. That turned out to be untrue (maybe
because `USER_ASM` is used?), so I missed zlib.

I noticed during code review of openzfs#4385 that the author of a
commit had peppered the various Makefile.am files with `$(TIRPC_LIBS)`
when putting it into lib/libspl/Makefile.am would have sufficed. Upon
further examination, it seems that he had copied what we do with
`$(ZLIB)`.

Autoconf is designed to propagate link dependencies to the binary into
which the static object is incorporated and the ELF interpreter will
load it for that binary without a problem.  If we specify it multiple
times, we end up with the situation where unless `LDFLAGS` contains
`-Wl,--as-needed`, the ELF interpreter is told to load the same library
multiple times, which is serves to load down ELF intialization at best
and will lead to unnecessary libraries being loaded at worst.

We should clean this up to provide a better example of how the code
should be written to future contributors. Otherwise, they will be
inclined to make the same mistake (for consistency) and we will end up
loading libraries that we do not need.

Signed-off-by: Richard Yao <[email protected]>
  • Loading branch information
ryao committed Mar 16, 2016
1 parent c352ec2 commit 473317d
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 8 deletions.
2 changes: 0 additions & 2 deletions cmd/zdb/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ zdb_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zdb_LDADD += $(ZLIB)
1 change: 0 additions & 1 deletion cmd/zfs/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ zfs_LDADD = \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zfs_LDADD += $(ZLIB)
zfs_LDFLAGS = -pthread
2 changes: 0 additions & 2 deletions cmd/zhack/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@ zhack_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zhack_LDADD += $(ZLIB)
2 changes: 0 additions & 2 deletions cmd/zstreamdump/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@ zstreamdump_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zstreamdump_LDADD += $(ZLIB)
2 changes: 1 addition & 1 deletion lib/libefi/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ nodist_libefi_la_SOURCES = \
$(USER_C) \
$(KERNEL_C)

libefi_la_LIBADD = $(LIBUUID) $(ZLIB)
libefi_la_LIBADD = $(LIBUUID)

EXTRA_DIST = $(USER_C)

0 comments on commit 473317d

Please sign in to comment.