On 2021-01-06 3:16 PM, Petr Machata wrote:
Roi Dayan <r...@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).
right ok.
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?
With is_json_context() I cannot check the type passed by the caller.
i.e. PRINT_JSON, PRINT_FP, PRINT_ANY.
From what I see now callers use is_json_context() to decide which print
to use. but in print_color_size() I should check the type to decide
which print.
