8000 Remove redundant NULL pointer checks by rimbi · Pull Request #853 · dimitri/pgcopydb · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove redundant NULL pointer checks #853

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

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

rimbi
Copy link
Contributor
@rimbi rimbi commented Jul 23, 2024

This is a cppcheck finding.

@rimbi rimbi requested a review from hanefi July 23, 2024 08:21
@rimbi rimbi self-assigned this Jul 23, 2024
Comment on lines -1682 to -1687
if (ptr == NULL)
{
log_error("Failed to tokenize Archive Item line: %s", line);
return false;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be happier if you briefly shared why you wanted to remove this check here.

If we keep this check here, we'd have guarantees that there are some more characters after the token that we parsed. I do not really see why we need this check, but I am unsure if it makes sense to remove these lines here.

Copy link
Contributor Author
@rimbi rimbi Jul 23, 2024

Choose a reason for hiding this comment

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

I see. The reason is that there is no way for ptr to be NULL. If it is NULL, then it means that line is also NULL, which means that the preceding isdigit(*line) ends with an error. Once ptr is not NULL, then it can't become NULL by incrementing it with ptr++.

@rimbi rimbi requested a review from hanefi July 23, 2024 14:23
@rimbi rimbi merged commit e0123a6 into dimitri:main Jul 23, 2024
19 checks passed
@rimbi rimbi deleted the cppcheck-nullPointerRedundantCheck branch July 23, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0