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

Clang's scan-base warning fixes #85

Closed
wants to merge 12 commits into from
Closed

Clang's scan-base warning fixes #85

wants to merge 12 commits into from

Conversation

proh14
Copy link
Contributor

@proh14 proh14 commented Oct 8, 2024

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 proh14 mentioned this pull request Oct 8, 2024
@proh14 proh14 changed the title clang's scan-base warning fixes Clang's scan-base warning fixes Oct 8, 2024
@proh14
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 8, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 9, 2024

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;

@adsr
Copy link
Owner

adsr commented Oct 10, 2024

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;

@proh14
Copy link
Contributor Author

proh14 commented Oct 13, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 13, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 23, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 23, 2024

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

@proh14
Copy link
Contributor Author

proh14 commented Oct 23, 2024

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

@adsr
Copy link
Owner

adsr commented Oct 25, 2024

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

@proh14 proh14 marked this pull request as ready for review October 25, 2024 03:57
@proh14
Copy link
Contributor Author

proh14 commented Oct 25, 2024

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
Copy link
Contributor Author

proh14 commented Oct 28, 2024

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

@adsr
Copy link
Owner

adsr commented Oct 30, 2024

Thanks again @proh14. I'll review soon.

@adsr
Copy link
Owner

adsr commented Nov 15, 2024

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

@adsr adsr closed this Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants