-
-
Notifications
You must be signed in to change notification settings - Fork 7k
checksrc: reduce directory-specific exceptions #18823
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
Conversation
docs/examples/multi-event.c
Outdated
| CURL *handle; | ||
|
|
||
| snprintf(filename, 50, "%d.download", num); | ||
| curl_msnprintf(filename, 50, "%d.download", num); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to use curl_msnprintf in the examples? The docs for them discourage use in new applications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I think they make the examples more complex than necessary...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was already used in a couple places (on a quick glance: for no strong reasons),
and I tried avoiding snprintf() that is banned elsewhere (and by default after this
patch), and for being traditionally problematic with MSVC.
But, no issues allowlisting snprintf here and dropping existing curl printf uses
(and perhaps disallow them in examples to keep it this way.)
I'll fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snprintf() didn't exist before VS2015 (but there is an _snprintf()):
https://ci.appveyor.com/project/curlorg/curl/builds/52850387/job/jbkhnbvt3hpb742l
curl_msnprintf() made it work before. The snprintf cases that I changed
were all in COMPLICATED_EXAMPLES and not built in CI, though one of
them is written for Windows (synctime).
I added a <VS2015-specific workaround. And I'm thinking it'd be nice to
build some of those complicated examples. At least synctime.
edit: the fun never ends, after fixing an obscure "can't use == with
floats" warning, it turns out a call is missing from UWP. So need to detect
UWP and do a fallback!
Fixing: ``` Unmatched ) in regex; marked by <-- HERE in m/ \*buffer_len = \(ssize_t) <-- HERE strtol\(/ at /home/runner/work/curl/curl/scripts/checksrc.pl line 916, <$R> line 380. ``` Ref: https://github.com/curl/curl/actions/runs/18209824275/job/51848079550#step:3:5 Also add a test case. Follow-up to 684f4cd #18779 Cherry-picked from #18823 Closes #18836
This reverts commit 8e4cea6.
also replace magic numbers with sizeof().
They are discouraged in user code.
```
docs/examples/synctime.c:312:39: error: comparing floating-point with '==' or '!=' is unsafe [-Werror=float-equal]
312 | if((double)(tzonediffWord * 3600) == tzonediffFloat)
| ^~
```
Both may apply to rare non-WinCE Windows builds too. - fix gcc 4.4.0 preprocessor error: ``` docs/examples/http2-upload.c:43:8: error: "_MSC_VER" is not defined ``` Ref: https://github.com/curl/curl/actions/runs/18238150607/job/51935502616 - fix wrong header order: Inlcude `windows.h` after `winsock2.h` via `curl/curl.h`. Regressions from 45438c8 #18823 Closes #18843
Also:
- tests/server: replace local `sstrerror()` with `curlx_strerror()`.
- tests/server: show the error code next to the string, where missing.
- curlx: use `curl_msnprintf()` when building for src and tests.
(units was already using it.)
- lib: drop unused includes found along the way.
- curlx_strerror(): avoid compiler warning (and another similar one):
```
In file included from servers.c:14:
../../lib/../../lib/curlx/strerr.c: In function ‘curlx_strerror’:
../../lib/../../lib/curlx/strerr.c:328:32: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
328 | SNPRINTF(buf, buflen, "%s", msg);
| ^
../../lib/../../lib/curlx/strerr.c:47:18: note: ‘snprintf’ output 1 or more bytes (assuming 2) into a destination of size 1
47 | #define SNPRINTF snprintf
| ^
../../lib/../../lib/curlx/strerr.c:328:7: note: in expansion of macro ‘SNPRINTF’
328 | SNPRINTF(buf, buflen, "%s", msg);
| ^~~~~~~~
```
Follow-up to 45438c8 #18823
Closes #18840
By making them defaults, then fixing and/or reshuffling remaining
exceptions as necessary.
snprintf,vsnprintf,sscanf,strtol.strtolwithatoito avoid a checksrc exception.strtolwithatol.strtolwithatol.strtoulwithatol/atoi.util_ultous.vsnprint->vsnprintf.Also:
Add
snprintfworkaround for <VS2015.-Wfloat-equal.strerror(). → curlx: move Curl_strerror, use in src and tests, banstrerrorglobally #18840strtoul()/strtol()tocurlx_str_hex(). → tests/server: replace banned functions withcurlx_str_hex#18837