All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: Roi Dayan <roid@nvidia.com>
Cc: <netdev@vger.kernel.org>, David Ahern <dsahern@gmail.com>,
	Petr Machata <me@pmachata.org>
Subject: Re: [PATCH iproute2] build: Fix link errors on some systems
Date: Wed, 6 Jan 2021 14:16:49 +0100	[thread overview]
Message-ID: <87y2h6urwe.fsf@nvidia.com> (raw)
In-Reply-To: <4a07fbc9-8e1c-ecd6-ee9e-31d1a952ba42@nvidia.com>


Roi Dayan <roid@nvidia.com> writes:

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

You don't need json_print_math.h IMHO, it can all be backed by the same
header, just different implementation modules. From the API point of
view, I don't think the user should really care which of the symbols use
math (though of course they will have to know whether to link in -lm).

Regarding the publishing, the _jw reference can be changed to a call to
is_json_context(), which does the same thing. Then _jw can stay private
in json_print.c.

Exposing an _IS_JSON_CONTEXT / _IS_FP_CONTEXT might be odd on account of
the initial underscore, but since it's only used in implementations,
maybe it's OK?

  reply	other threads:[~2021-01-06 13:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-30 19:11 [PATCH iproute2] build: Fix link errors on some systems Roi Dayan
2021-01-04 16:07 ` Petr Machata
2021-01-06  8:42   ` Roi Dayan
2021-01-06 12:51     ` Roi Dayan
2021-01-06 13:16       ` Petr Machata [this message]
2021-01-06 14:20         ` Roi Dayan
2021-01-06 14:24           ` Petr Machata
2021-01-07  6:52             ` Roi Dayan
2021-01-06 16:00     ` Stephen Hemminger
2021-01-06 16:08       ` Petr Machata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y2h6urwe.fsf@nvidia.com \
    --to=petrm@nvidia.com \
    --cc=dsahern@gmail.com \
    --cc=me@pmachata.org \
    --cc=netdev@vger.kernel.org \
    --cc=roid@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.