Skip to content

Commit

Permalink
Only libzpool must link to zlib
Browse files Browse the repository at this point in the history
I noticed during code review of openzfs#4385 that the author 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), which
unfortunately is wrong because it is only needed in the Makefile.am
corresponding to libzpool.

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.

The worst case scenario does not happen with zlib because even with
-Wl,--as-needed, it is loaded anyway for libuutil. However, 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 15, 2016
1 parent c352ec2 commit 3dc3066
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 3dc3066

Please sign in to comment.