Skip to content

clang-tidy: avoid assigments in if expressions#21256

Closed
vszakats wants to merge 4 commits intocurl:masterfrom
vszakats:if-assigments
Closed

clang-tidy: avoid assigments in if expressions#21256
vszakats wants to merge 4 commits intocurl:masterfrom
vszakats:if-assigments

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Apr 7, 2026

Also enable check in clang-tidy.

Cherry-picked from #20794


https://github.com/curl/curl/pull/21256/files?w=1

vszakats added 4 commits April 7, 2026 16:34
```
/home/runner/work/curl/curl/lib/vtls/x509asn1.c:163:15: error: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition,-warnings-as-errors]
  163 |   else if(!(b &= 0x7F)) {
      |             ~~^~~~~~~
/home/runner/work/curl/curl/lib/vtls/x509asn1.c:163:15: note: if it should be an assignment, move it out of the 'if' condition
/home/runner/work/curl/curl/lib/vtls/x509asn1.c:163:15: note: if it is meant to be an equality check, change '=' to '=='
```
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tidies up a couple of code paths to avoid assignments inside if conditions, and enables the corresponding clang-tidy check to enforce the pattern going forward.

Changes:

  • Refactor ASN.1 element length parsing to avoid b &= ... inside an if condition.
  • Refactor GnuTLS load_file() error handling to avoid assignments within a compound if.
  • Enable bugprone-assignment-in-if-condition in .clang-tidy.yml.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
lib/vtls/x509asn1.c Reworks ASN.1 length parsing control flow to avoid assignment-in-condition.
lib/vtls/gtls.c Rewrites load_file() checks into stepwise validation with goto out.
.clang-tidy.yml Enables clang-tidy check to flag assignment in if conditions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/vtls/gtls.c
Comment on lines +208 to +214
filelen = ftell(f);
if(filelen < 0)
goto out;
if(fseek(f, 0, SEEK_SET) != 0)
goto out;
ptr = curlx_malloc((size_t)filelen);
if(!ptr)
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

filelen is a long from ftell(), but it gets cast to size_t for allocation/read and later stored into loaded_file.size (a gnutls_datum_t size field, typically unsigned int). If filelen exceeds UINT_MAX (or SIZE_MAX), this will truncate the reported size and/or attempt an oversized allocation. Consider adding an explicit upper-bound check (e.g., fail/return empty when filelen > (long)UINT_MAX) before curlx_malloc().

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

unrelated. separate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#21257

@vszakats vszakats changed the title tidy-up: avoid assigments in if expressions clang-tidy: avoid assigments in if expressions Apr 7, 2026
@vszakats vszakats closed this in d3dc5db Apr 7, 2026
@vszakats vszakats deleted the if-assigments branch April 7, 2026 14:59
vszakats added a commit to vszakats/curl that referenced this pull request Apr 7, 2026
vszakats added a commit that referenced this pull request Apr 7, 2026
Used for issuer certs. Limit the size at `CURL_MAX_INPUT_LENGTH`, 8MB.

Bug: #21256 (comment)

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

Labels

Development

Successfully merging this pull request may close these issues.

2 participants