Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: Avoid an extern variable by using sss_chain_id_get() #5735

Closed
wants to merge 1 commit into from

Conversation

cryptomilk
Copy link

No description provided.

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Aug 4, 2021

In general this feels cleaner but this would require inclusion of sss_chain_id.c to libsss_debug:

libsss_debug.so: undefined reference to `sss_chain_id_get'

and this, in turn, would require linking against libtevent (not the case currently and probably better to be avoided as libsss_debug is used in different helpers that doesn't link against libtevent)

@alexey-tikhonov
Copy link
Member

Other functions in sss_chain_id.c still need to access debug_chain_id

src/util/debug.c Outdated Show resolved Hide resolved
@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Aug 4, 2021

I guess include of stdint.h is missing in debug.h

But, in general, I'm not sure the split of sss_chain_id_* functions to different files is much better then single extern var...

Signed-off-by: Andreas Schneider <asn@redhat.com>
@pbrezina
Copy link
Member

We already have multiple extern debug variables. Removing this one with the cost of splitting the implementation seems as unnecessary paranoia. Additionally, the main reason that I access the variable directly inside sss_chain_id.c is to avoid unnecessary functions calls since it has little impact in performance (negligible that's true, but easily avoided) .

I'll leave it to others to decide. Also _get should be inlined.

@cryptomilk
Copy link
Author

Then lets just close it ... :-)

@cryptomilk cryptomilk closed this Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants