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

building without asserts #5080

Open
chu11 opened this issue Apr 9, 2023 · 1 comment
Open

building without asserts #5080

chu11 opened this issue Apr 9, 2023 · 1 comment

Comments

@chu11
Copy link
Member

chu11 commented Apr 9, 2023

After comments in #5079, I was just curious if flux-core could be built without asserts and if the tests would pass.

There are a number of "unused-but-set-variable" and "unused-variable" errors and some stupid corner cases (e.g. I had a function in libsubprocess that ended in an assert, so got a warning that function didn't return a value). But nothing super horrible (i.e. assert (rv = something); return rv; would be really really bad). Tests pass.

I just thought I'd log my diff here just for note taking purposes and in case anyone in the future wanders into this/needs it. I think we're a long ways off from wanting to build flux without asserts in the general case.

diff --git a/src/broker/Makefile.am b/src/broker/Makefile.am
index 4902119..41a9b88 100644
--- a/src/broker/Makefile.am
+++ b/src/broker/Makefile.am
@@ -1,6 +1,9 @@
 AM_CFLAGS = \
        $(WARNING_CFLAGS) \
-       $(CODE_COVERAGE_CFLAGS)
+       $(CODE_COVERAGE_CFLAGS) \
+       -Wno-error=unused-but-set-variable \
+       -Wno-error=unused-variable
+
 AM_LDFLAGS = \
        $(CODE_COVERAGE_LIBS)
 
diff --git a/src/cmd/Makefile.am b/src/cmd/Makefile.am
index 853d987..3e6eff4 100644
--- a/src/cmd/Makefile.am
+++ b/src/cmd/Makefile.am
@@ -1,6 +1,7 @@
 AM_CFLAGS = \
        $(WARNING_CFLAGS) \
-       $(CODE_COVERAGE_CFLAGS)
+       $(CODE_COVERAGE_CFLAGS) \
+       -Wno-error=unused-but-set-variable
 
 AM_LDFLAGS = \
        $(CODE_COVERAGE_LIBS)
diff --git a/src/common/libflux/Makefile.am b/src/common/libflux/Makefile.am
index 1e184ca..c3be6ea 100644
--- a/src/common/libflux/Makefile.am
+++ b/src/common/libflux/Makefile.am
@@ -1,6 +1,7 @@
 AM_CFLAGS = \
        $(WARNING_CFLAGS) \
-       $(CODE_COVERAGE_CFLAGS)
+       $(CODE_COVERAGE_CFLAGS) \
+       -Wno-error=unused-but-set-variable
 
 AM_LDFLAGS = \
        $(CODE_COVERAGE_LIBS)
diff --git a/src/common/liblsd/Makefile.am b/src/common/liblsd/Makefile.am
index 3670604..c159164 100644
--- a/src/common/liblsd/Makefile.am
+++ b/src/common/liblsd/Makefile.am
@@ -1,7 +1,9 @@
 AM_CFLAGS = \
        $(WARNING_CFLAGS) \
        $(CODE_COVERAGE_CFLAGS) \
-       -Wno-parentheses -Wno-error=parentheses
+       -Wno-parentheses \
+       -Wno-error=parentheses \
+       -Wno-error=unused-but-set-variable
 
 AM_LDFLAGS = \
        $(CODE_COVERAGE_LIBS)
diff --git a/src/common/libsubprocess/subprocess.c b/src/common/libsubprocess/subprocess.c
index fe1df7e..4e72c95 100644
--- a/src/common/libsubprocess/subprocess.c
+++ b/src/common/libsubprocess/subprocess.c
@@ -327,6 +327,7 @@ static flux_subprocess_state_t state_change_next (flux_subprocess_t *p)
 
     /* shouldn't be possible to reach here */
     assert (0);
+    return 0;
 }
 
 static void state_change_check_cb (flux_reactor_t *r,
@@ -748,6 +749,7 @@ int flux_subprocess_stream_status (flux_subprocess_t *p, const char *stream)
     else {
         /* fb = c->read_buffer; */
         assert (0);
+        ret = -1;
     }
 
     return ret;
diff --git a/src/modules/kvs/Makefile.am b/src/modules/kvs/Makefile.am
index fad7838..ec5ddc0 100644
--- a/src/modules/kvs/Makefile.am
+++ b/src/modules/kvs/Makefile.am
@@ -1,6 +1,7 @@
 AM_CFLAGS = \
        $(WARNING_CFLAGS) \
-       $(CODE_COVERAGE_CFLAGS)
+       $(CODE_COVERAGE_CFLAGS) \
+       -Wno-error=unused-but-set-variable
 
 AM_LDFLAGS = \
        $(CODE_COVERAGE_LIBS)
diff --git a/t/Makefile.am b/t/Makefile.am
index e9d9d99..d14c684 100644
--- a/t/Makefile.am
+++ b/t/Makefile.am
@@ -1,6 +1,7 @@
 AM_CFLAGS = \
        $(WARNING_CFLAGS) \
-       $(CODE_COVERAGE_CFLAGS)
+       $(CODE_COVERAGE_CFLAGS) \
+       -Wno-error=unused-but-set-variable
 
 AM_LDFLAGS = \
        $(CODE_COVERAGE_LIBS)
@chu11
Copy link
Member Author

chu11 commented Apr 9, 2023

now that i think about it, should this be in discussions?

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

No branches or pull requests

1 participant