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.

Sign up for GitHub

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

Jump to bottom

lib: fix gcc warning in printf call #12082

Closed
wants to merge 1 commit into from
Closed

lib: fix gcc warning in printf call #12082

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 11, 2023 •
edited

Do not pass NULL to printf %s.

Seen with gcc 13.2.0 on Debian:

.../curl/lib/connect.c:696:27: warning: '%s' directive argument is null [-Wformat-overflow=]

Ref: https://github.com/curl/curl-for-win/actions/runs/6476161689/job/17584426483#step:3:11104

Ref: #10284
Co-authored-by: Jay Satiro
Closes #12082

In function 'is_connected',
    inlined from 'cf_he_connect' at /home/runner/work/curl-for-win/curl-for-win/curl/lib/connect.c:906:16:
/home/runner/work/curl-for-win/curl-for-win/curl/lib/connect.c:696:27: warning: '%s' directive argument is null [-Wformat-overflow=]
  696 |     CURL_TRC_CF(data, cf, "%s assess started=%d, result=%d",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/curl-for-win/curl-for-win/curl/lib/curl_trc.h:123:38: note: in definition of macro 'CURL_TRC_CF'
  123 |          Curl_trc_cf_infof(data, cf, __VA_ARGS__); } while(0)
      |                                      ^~~~~~~~~~~
In file included from /home/runner/work/curl-for-win/curl-for-win/curl/bld/lib/CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c:49:
/home/runner/work/curl-for-win/curl-for-win/curl/lib/connect.c: In function 'cf_he_connect':
/home/runner/work/curl-for-win/curl-for-win/curl/lib/connect.c:696:28: note: format string is defined here
  696 |     CURL_TRC_CF(data, cf, "%s assess started=%d, result=%d",
      |                            ^~
make[2]: Leaving directory '/home/runner/work/curl-for-win/curl-for-win/curl/bld'

@vszakats vszakats mentioned this pull request Oct 11, 2023
Closed
jay
jay reviewed Oct 11, 2023
View reviewed changes
lib/connect.c Outdated Show resolved Hide resolved
jay
jay approved these changes Oct 11, 2023
View reviewed changes
Copy link
Member

@jay jay left a comment •
edited

Choose a reason for hiding this comment

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

Since the warning text is not in this one I would add a brief explanation of it or the action taken, example

lib: fix gcc warning

- Do not pass NULL to printf %s.

...

The problem with the warnings in some cases like this one is they are confusing and take extra cognitive processing so that's why I was suggesting a summary of sorts so it's easy to see what the warning is actually for or what is being fixed. I agree it is good to keep the actual warning in the PR so we have a record of what it actually said. The CI links are all ephemeral so in the commit message they are only good for so long.

@vszakats vszakats closed this in 4e57d0f Oct 13, 2023
@vszakats vszakats changed the title lib: fix gcc warning lib: fix gcc warning in printf call Oct 13, 2023
@vszakats vszakats deleted the warn3 branch October 13, 2023 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jay jay jay approved these changes

Assignees
No one assigned
Labels
connecting & proxies tidy-up
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@vszakats @jay