mle icon indicating copy to clipboard operation
mle copied to clipboard

Clang's scan-base warning fixes

Open proh14 opened this issue 1 year ago • 16 comments

Hello :-)

I'm working on all the warnings in this draft #84, each fix will be a single commit(tell me if you don't like that).

If you see any problems with the fixes, consider mentioning it here!

image

proh14 avatar Oct 08 '24 04:10 proh14

Commit 1: Fixes dead assignment at util.c, line 278.

proh14 avatar Oct 08 '24 04:10 proh14

Commit 2: Fixes dead assignment at cmd.c, line 523

proh14 avatar Oct 08 '24 07:10 proh14

Commit 3: Fixes nonnull warning at util.c, line 542.

Kinda stupid, I guess the static analyzer doesn't understand ref_cap and cap

proh14 avatar Oct 08 '24 19:10 proh14

Commit 4: Fixes dead assignment at cmd.c, line 1621.

proh14 avatar Oct 08 '24 20:10 proh14

Commit 5: Fixes dead assignment at buffer.c, line 1484.

proh14 avatar Oct 08 '24 23:10 proh14

Commit 6: Fixes dead increment at cmd.c, line 1904.

proh14 avatar Oct 08 '24 23:10 proh14

Commit 7: Fixes dead nested assignment at buffer.c, line 61.

proh14 avatar Oct 08 '24 23:10 proh14

I have a question,

If you look at these lines

// Return a line given a line_index
int buffer_get_bline(buffer_t *self, bint_t line_index, bline_t **ret_bline) {
    return buffer_get_bline_w_hint(self, line_index, self->first_line, ret_bline);
}

// Return a line given a line_index, starting at hint and iterating outward
int buffer_get_bline_w_hint(buffer_t *self, bint_t line_index, bline_t *opt_hint, bline_t **ret_bline) {
    bline_t *fwd, *rev, *found;
    MLBUF_MAKE_GT_EQ0(line_index);

    if (!opt_hint) {
        opt_hint = self->first_line;
    }

    fwd = opt_hint;
    rev = opt_hint->prev;

you see when this line is executed: return buffer_get_bline_w_hint(self, line_index, self->first_line, ret_bline)

opt_hint will be set to whatever self->first_line is.

    if (!opt_hint) {
        opt_hint = self->first_line;
    }

Here if statement checks if opt_hint(self->first_line) is null, if it is, it set's it to self->first_line, so that will make opt_hint null and then you are de-referencing it. I don't know how I can fix this. It seems like you accidentally made opt_hint self->first_line instead of another field in self

I guess you just:

return MLBUF_ERR;

proh14 avatar Oct 09 '24 00:10 proh14

Yes, returning an error is OK in that case. self->first_line should never be non-NULL if the library is being used properly. To guard against a segfault if someone passes in an empty struct / to satisfy the static analyzer, something like this should be OK:

diff --git a/buffer.c b/buffer.c
index d85466d..ee5a1dd 100644
--- a/buffer.c
+++ b/buffer.c
@@ -693,7 +693,7 @@ int buffer_replace_w_bline(buffer_t *self, bline_t *start_line, bint_t start_col
 
 // Return a line given a line_index
 int buffer_get_bline(buffer_t *self, bint_t line_index, bline_t **ret_bline) {
-    return buffer_get_bline_w_hint(self, line_index, self->first_line, ret_bline);
+    return buffer_get_bline_w_hint(self, line_index, NULL, ret_bline);
 }
 
 // Return a line given a line_index, starting at hint and iterating outward
@@ -703,6 +703,10 @@ int buffer_get_bline_w_hint(buffer_t *self, bint_t line_index, bline_t *opt_hint
 
     if (!opt_hint) {
         opt_hint = self->first_line;
+        if (!opt_hint) {
+            *ret_bline = NULL;
+            return MLBUF_ERR;
+        }
     }
 
     fwd = opt_hint;

adsr avatar Oct 10 '24 23:10 adsr

Commit 8: Fixes potential null-pointer de-reference in buffer.c, line 709.

proh14 avatar Oct 13 '24 01:10 proh14

Commit 9: Fixes potential null-pointer de-reference in editor.c, line 1518.

proh14 avatar Oct 13 '24 01:10 proh14

Commit 10: Fixes potential null-pointer de-reference in editor.c, line 650.

proh14 avatar Oct 23 '24 06:10 proh14

Commit 11: Fixes potential memory leak in editor.c, line 2272.

proh14 avatar Oct 23 '24 06:10 proh14

Commit 12: Fixes potential zero size memory allocation in cmd.c, line 553.

proh14 avatar Oct 23 '24 06:10 proh14

@proh14 Let me know when you'd like me to review / merge. No rush. Your call. Thank you again for your contributions.

adsr avatar Oct 25 '24 01:10 adsr

Hello :), I've been taking look at the last 3 problems but they all seem to be false flags that can't be really fixed. I will mark this ready for review so you can take a look at what I did.

proh14 avatar Oct 25 '24 03:10 proh14

@adsr if you can please give it the hacktoberfest-accepted tag I need it for hacktoberfest

proh14 avatar Oct 28 '24 00:10 proh14

Thanks again @proh14. I'll review soon.

adsr avatar Oct 30 '24 04:10 adsr

All merged. Just neatened 1 or 2 things up. Thank you again @proh14.

adsr avatar Nov 15 '24 01:11 adsr