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

Fix bugs identified by running clang's scan-build-16 on the MLVWM source #48

Open
morgant opened this issue Aug 31, 2024 · 13 comments
Open
Assignees
Labels
Milestone

Comments

@morgant
Copy link
Owner

morgant commented Aug 31, 2024

I recently learned about Clang's scan-build tool for identifying bugs and ran the following on my OpenBSD amd64/7.5-stable workstation:

scan-build-16 -o tmp/scan-build make -j4

Which identified 29 bugs, including:

  • (x6) Dereference of null pointer
  • (x1) Garbage return value
  • (x1) Result of operation is garbage or undefined
  • (x1) Uninitialized argument value
  • (x1) Unix API
  • (x1) Memory leak
  • (x2) Allocator sizeof operand mismatch
  • (x14) Dead assignment
  • (x2) Dead nested assignment

mlvwm - scan-build results.pdf

@morgant morgant added the bug label Aug 31, 2024
@morgant morgant added this to the 0.9.5 milestone Aug 31, 2024
@morgant morgant self-assigned this Aug 31, 2024
@morgant
Copy link
Owner Author

morgant commented Aug 31, 2024

I have addressed all the "dead assignments" identified in borders.c, menus.c, and event.c with the exception of baseHeight & baseWidth in event.c's ConstrainSize() as they do appear to be used. For the those, I'm wondering if it's a parsing issue since the formatting is quite wonky and there's a possibility that there are shift-jis characters in there.

@morgant
Copy link
Owner Author

morgant commented Aug 31, 2024

I'm also skipping the "dead nested assignments" identified in event.c for similar reasons.

morgant added a commit that referenced this issue Aug 31, 2024
… can't be found, as identified by clang scan-build. Issue #48
@morgant
Copy link
Owner Author

morgant commented Aug 31, 2024

I have fixed a memory leak in misc.c's ReadIcon() due to a new Icon being allocated before checking to see if the icon file at the path actually exists.

morgant added a commit that referenced this issue Sep 1, 2024
…Label() as identified by clang scan-build. Issue #48
@morgant
Copy link
Owner Author

morgant commented Sep 1, 2024

I have fixed a NULL value being passed to strcmp() in menus.c's ChangeMenuLabel().

morgant added a commit that referenced this issue Sep 2, 2024
@morgant
Copy link
Owner Author

morgant commented Sep 2, 2024

I have fixed a possible uninitialized argument value in misc.c's ReadIcon().

morgant added a commit that referenced this issue Sep 2, 2024
…ess_menu() & ChoiseMenu(), as identified by clang scan-build. Issue #48
@morgant
Copy link
Owner Author

morgant commented Sep 2, 2024

I have fixed all the potential dereference of null pointer values in menu.c's press_menu() & ChoiseMenu().

morgant added a commit that referenced this issue Sep 2, 2024
@morgant
Copy link
Owner Author

morgant commented Sep 2, 2024

Fixed a dereference of null pointer in misc.c's RaiseMlvwmWindow().

morgant added a commit that referenced this issue Sep 2, 2024
…r type in config.c's SetDesktopNum(), as identified by clang scan-build. Issue #48
morgant added a commit that referenced this issue Sep 2, 2024
…identally allocate memory for a number of MlvwmWindows instead of just a number of pointers to MlvwmWindows for Scr.LastActive, as identified by clang scan-build. Issue #48
@morgant
Copy link
Owner Author

morgant commented Sep 2, 2024

Fixed "allocator sizeof operand mismatch" bugs in config.c & mlvwm.c where calloc() was being used to accidentally allocate memory for a number of MlvwmWindows instead of just a number of pointers to MlvwmWindows for Scr.LastActive. Scr.LastActive is an array of pointers to the "last active" window on each virtual desktop, so there may be multiple if there are multiple virtual desktops.

morgant added a commit that referenced this issue Sep 2, 2024
@morgant
Copy link
Owner Author

morgant commented Sep 2, 2024

Fixed potential garbage return value in event.c's NextActiveWin(), which is the last bug identified by clang's scan-build that I intend to fix at this time. The remaining "dead assignment" bugs in event.c's ConstrainSize() are not something that I feel need to be addressed.

@morgant
Copy link
Owner Author

morgant commented Sep 2, 2024

I'm going to test this branch for a bit before merging it.

morgant added a commit that referenced this issue Sep 3, 2024
@morgant
Copy link
Owner Author

morgant commented Sep 3, 2024

Clang was warning about an unused lp variable in menus.c's RedrawMenuBar() at compile time (not scan-build), so I figured I'd address it here as well.

@morgant
Copy link
Owner Author

morgant commented Sep 3, 2024

There are a couple more compile warnings that I'd like to address now too (related to Issue #6, replacing sprintf() with snprintf()):

mlvwm.c:386(mlvwm.o:(Done)): warning: strcpy() is almost always misused, please use strlcpy()
functions.c:322(functions.o:(ChangeDesk)): warning: sprintf() is often misused, please use snprintf()

Update: No need to address the second warning about sprintf() in functions.c, I clearly addressed that in a comment on Issue #6. Furthermore, I have split this out into Issue #49 so it can be addressed separately and more appropriately (esp. using libbsd under Linux.)

@morgant
Copy link
Owner Author

morgant commented Sep 3, 2024

I also took this opportunity to correct the spelling of ChoiseMenu() to ChooseMenu() in functions.c & menus.c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant