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

d.vect: Dereference after Null Check #4640

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ShubhamDesai
Copy link
Contributor

This pull request fixes issue identified by Coverity scan (CID : 1207395)
Changes Made:

  • Added a condition to check if Symb is NULL before using it.
  • If Symb is NULL, a G_warning is issued to inform the user that the symbol could not be displayed, preventing null dereference.

@github-actions github-actions bot added C Related code is in C module display labels Nov 3, 2024
@ShubhamDesai ShubhamDesai changed the title display/d.vect: Dereference after Null Check d.vect: Dereference after Null Check Nov 3, 2024
ShubhamDesai and others added 2 commits November 3, 2024 15:07
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

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

I added a minor suggestion below. Also, please address CID 1270355 (and perhaps 1207713) here too.

Comment on lines 348 to 349
else {
G_warning(_("Symbol is NULL; unable to display points"));
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this else statement (with warning). The only situation Symb is NULL is if the call Symb = S_read(symbol_name); in line 325 fails (in which case a warning is issued anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

display/d.vect/lines.c Outdated Show resolved Hide resolved
display/d.vect/lines.c Outdated Show resolved Hide resolved
ShubhamDesai and others added 2 commits December 1, 2024 21:20
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@ShubhamDesai
Copy link
Contributor Author

I added a minor suggestion below. Also, please address CID 1270355 (and perhaps 1207713) here too.

Done.
For CID 1270355 added just one check if (color)
For CID 1207713 added G_free(Symb)

@echoix
Copy link
Member

echoix commented Dec 22, 2024

@ShubhamDesai When you'll have the chance, could you take a look at why there's multiple test failures across OSes?

@ShubhamDesai
Copy link
Contributor Author

@ShubhamDesai When you'll have the chance, could you take a look at why there's multiple test failures across OSes?

yes

@echoix
Copy link
Member

echoix commented Dec 24, 2024

I still don't see where the -6 error comes from..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C display module
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants