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 cppcheck reports: #133

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@serval2412
Contributor

serval2412 commented Dec 31, 2014

[tests/libtest/lib1900.c:182]: (style) Array index 'handlenum' is used before limits check.
[docs/examples/sepheaders.c:65]: (error) Resource leak: headerfile

Fix cppcheck reports:
[tests/libtest/lib1900.c:182]: (style) Array index 'handlenum' is used before limits check.
[docs/examples/sepheaders.c:65]: (error) Resource leak: headerfile
@captain-caveman2k

This comment has been minimized.

Member

captain-caveman2k commented Dec 31, 2014

Hi Julien,

Many thanks.

I've pushed your modification to sepheaders.c but have a question for you about the modification to lib1900.c:

Is the check for 'handlenum < num_handles' needed in that second if statement, in order to pass cppcheck, given that the same check happens a few lines up and the values of 'handlenum' and 'num_handles' are not modified in between the two if statements?

Unless I've missed something I don't think it is needed from a logic point of view.

Kind Regards

Steve

@serval2412

This comment has been minimized.

Contributor

serval2412 commented Dec 31, 2014

You're right, I think cppcheck isn't smart enough to detect that the check already exists some line above.
In this case, you may perhaps remove the dup check in the line cppcheck reported?

@captain-caveman2k

This comment has been minimized.

Member

captain-caveman2k commented Dec 31, 2014

Hi again,

I think the check being in the second part of the second if statement may be confusing cppcheck as well :(

Are you able to test this theory and produce a patch if it is successful - as I don't run cppcheck personally?

Many thanks

Steve

@serval2412

This comment has been minimized.

Contributor

serval2412 commented Dec 31, 2014

Ok if I do this:

  •  if(msnow - mslast >= urltime[handlenum] && handlenum < num_handles) {
    
  •  if((msnow - mslast) >= urltime[handlenum]) {
    
    everything is alright for cppcheck.
@captain-caveman2k

This comment has been minimized.

Member

captain-caveman2k commented Dec 31, 2014

Cheers Julien,

I'll prepare a patch on your behalf and push it.

Fyi: Daniel has started to use Coverity to scan the source code over recent months and I've just been looking at the cppcheck website, whilst compiling various patches, so I will try cppcheck myself over the next few days.

Thanks again

Steve

@serval2412

This comment has been minimized.

Contributor

serval2412 commented Dec 31, 2014

Great!
cppcheck is really simple to use and is free!
cppcheck --enable=all
you can also produce html report, I use this for LibreOffice (see https://wiki.documentfoundation.org/Development/Cppcheck)

captain-caveman2k added a commit that referenced this pull request Dec 31, 2014

lib1900.c: Fixed cppcheck error
lib1900.c:182: (style) Array index 'handlenum' is used before limits
               check

Bug: #133
@captain-caveman2k

This comment has been minimized.

Member

captain-caveman2k commented Dec 31, 2014

Pushed as commit ee0941a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment