Re: [PATCH iproute2] build: Fix link errors on some systems

[Roi Dayan <roid@nvidia.com>]

On 2021-01-06 10:42 AM, Roi Dayan wrote:
> 
> 
> On 2021-01-04 6:07 PM, Petr Machata wrote:
>>
>> Roi Dayan <roid@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 <roid@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?