On 2021-01-06 10:42 AM, Roi Dayan wrote:


On 2021-01-04 6:07 PM, Petr Machata wrote:

Roi Dayan <r...@nvidia.com> writes:

Since moving get_rate() and get_size() from tc to lib, on some
systems we fail to link because of missing the math lib.
Move the link flag from tc makefile to the main makefile.

Hmm, yeah, it gets optimized out on x86-64. The issue is reproducible
on any platform with -O0.

../lib/libutil.a(utils.o): In function `get_rate':
utils.c:(.text+0x10dc): undefined reference to `floor'
../lib/libutil.a(utils.o): In function `get_size':
utils.c:(.text+0x1394): undefined reference to `floor'
../lib/libutil.a(json_print.o): In function `sprint_size':
json_print.c:(.text+0x14c0): undefined reference to `rint'
json_print.c:(.text+0x14f4): undefined reference to `rint'
json_print.c:(.text+0x157c): undefined reference to `rint'

Fixes: f3be0e6366ac ("lib: Move get_rate(), get_rate64() from tc here")
Fixes: 44396bdfcc0a ("lib: Move get_size() from tc here")
Fixes: adbe5de96662 ("lib: Move sprint_size() from tc here, add print_size()")
Signed-off-by: Roi Dayan <r...@nvidia.com>
---
  Makefile    | 1 +
  tc/Makefile | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e64c65992585..2a604ec58905 100644
--- a/Makefile
+++ b/Makefile
@@ -59,6 +59,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma dcb man
  LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
  LDLIBS += $(LIBNETLINK)
+LDFLAGS += -lm
  all: config.mk
      @set -e; \
diff --git a/tc/Makefile b/tc/Makefile
index 5a517af20b7c..8d91900716c1 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -110,7 +110,7 @@ ifneq ($(TC_CONFIG_NO_XT),y)
  endif
  TCOBJ += $(TCMODULES)
-LDLIBS += -L. -lm
+LDLIBS += -L.
  ifeq ($(SHARED_LIBS),y)
  LDLIBS += -ldl

This will work, but it will give a libm dependency to all the tools.
util.c currently tries not to do that:

    /* emulate ceil() without having to bring-in -lm and always be >= 1 */
    *val = t;
    if (*val < t)
        *val += 1;

I think that just adding an unnecessary -lm is more of a tidiness issue
than anything else. One way to avoid it is to split the -lm deps out
from util.c / json_print.c to like util_math.c / json_print_math.c. That
way they will be in an .o of their own, and won't be linked in unless
the binary in question needs the code. Then the binaries that do call it
can keep on linking in -lm like they did so far.

Thoughts?


ok fine by me.

I looked at this and for get_size()/rate/.. it went smooth.
but for print_color_size() there is an issue that it uses
_IS_JSON_CONTEXT and statuic *_jw which are defined in json_print.c
Is it ok to expose those in json_print.h now so json_print_math.c
could use?

Reply via email to