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

Feature/gcc warning errors #23

Merged
merged 3 commits into from
Nov 7, 2023

Conversation

badevos
Copy link

@badevos badevos commented Nov 6, 2023

Hi,

I'm trying to integrate libosdp into another project, which is using gcc 8.4.0 with '-Werror'.
This has caused a few issues which can easily be resolved I guess.

In case you accept these changes, I can update the PR for libosdp, ignoring the submodule pointing to my fork.

Let me know your thoughts.

Thanks in advance.

Kr,
Bart.

When compiling with gcc 8.4.0 with -Werror, the following error is
thrown:

src/logger.c:72:40: error: self-comparison always evaluates to true [-Werror=tautological-compare]
  while (*p != '\0' && *q != '\0' && *p == *p) {
                                        ^~
cc1: some warnings being treated as errors

It is assumed that *p needs to be compared with *q
Copy link
Member

@sidcha sidcha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me except for the _GNU_SOURCE part. Thanks for the PR.

src/logger.c Outdated
Comment on lines 6 to 8
#ifndef _GNU_SOURCE
#define _GNU_SOURCE /* See feature_test_macros(7) */
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick look, we are not using anything from _GNU_SOURCE. Without this, macro both libosdp and c-utils builds fine for me. Can you please check and remove _GNU_SOURCE here and in libosdp?

@@ -69,7 +70,7 @@ static const char *get_rel_path(logger_t *ctx, const char *abs_path)

p = ctx->root_path;
q = abs_path;
while (*p != '\0' && *q != '\0' && *p == *p) {
while (*p != '\0' && *q != '\0' && *p == *q) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thank you.

Using gcc 8.4.0 throws this warning:

libosdp/utils/include/utils/utils.h:31: warning: "BIT" redefined
 #define BIT(n)                         (1ull << (n))

It's safe to conditionally define this
Using gcc 8.4.0 throws this warning:

libosdp/utils/src/logger.c:7: warning: "_GNU_SOURCE" redefined
 #define _GNU_SOURCE  /* See feature_test_macros(7) */

However there is no need to use _GNU_SOURCE, it's safe to remove it.
@badevos
Copy link
Author

badevos commented Nov 7, 2023

I have reordered my commits and changed the one on _GNU_SOURCE. As my environment sets _GNU_SOURCE, it's okay for me to remove it.
As soon as you have merged it, I will update the main repo of libosdp as you requested

@sidcha sidcha merged commit cd90eef into goToMain:master Nov 7, 2023
1 of 5 checks passed
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.

2 participants