Skip to content

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Oct 3, 2025

By making them defaults, then fixing and/or reshuffling remaining
exceptions as necessary.

  • checksrc: ban by default: snprintf, vsnprintf, sscanf, strtol.
  • examples: replace strtol with atoi to avoid a checksrc exception.
  • tests/libtest: replace strtol with atol.
  • tests/server: replace most strtol with atol.
  • tests/server: replace most strtoul with atol/atoi.
  • tests/server: drop no longer used util_ultous.
  • fix typo in checksrc rules: vsnprint -> vsnprintf.
  • update local exceptions.

Also:

  • examples: ban curl printf functions. They're discouraged in user code.
  • examples: replace curl printf with system printf.
    Add snprintf workaround for <VS2015.
  • examples/synctime: fix -Wfloat-equal.
  • examples/synctime: exclude for non-Windows and non-UWP Windows.
  • examples/synctime: build by default.

CURL *handle;

snprintf(filename, 50, "%d.download", num);
curl_msnprintf(filename, 50, "%d.download", num);
Copy link
Contributor

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

Copy link
Member

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...

Copy link
Member Author

@vszakats vszakats Oct 3, 2025

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.

Copy link
Member Author

@vszakats vszakats Oct 3, 2025

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!

vszakats added a commit that referenced this pull request Oct 3, 2025
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
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)
      |                                       ^~
```
@vszakats vszakats changed the title checksrc: tidy up banned function exceptions checksrc: reduce directory-specific exceptions Oct 3, 2025
@vszakats vszakats marked this pull request as ready for review October 3, 2025 21:27
@vszakats vszakats closed this in 45438c8 Oct 3, 2025
@vszakats vszakats deleted the d-snprintf branch October 3, 2025 22:49
vszakats added a commit to vszakats/curl that referenced this pull request Oct 4, 2025
vszakats added a commit that referenced this pull request Oct 4, 2025
vszakats added a commit that referenced this pull request Oct 4, 2025
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
vszakats added a commit that referenced this pull request Oct 6, 2025
Replace an `strtol()` and `strtoul()` call, both used in hex mode, with
`curlx_str_hex()`.

Follow-up to 45438c8 #18823

Closes #18837
vszakats added a commit that referenced this pull request Oct 6, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants