OS/400 ascii fputc wrapper deviates from the posix standard by the
fact that it returns the ebcdic encoding of the original ascii
character. Testing for a matching value for success will then always
fail.
This commit replaces the chariacter comparison by an explicit error
return check.
Follow-up to ef2cf58Closes#13367
This fixes miscellaneous typos and duplicated words in the docs, lib
and test comments and a few user facing errorstrings.
Author: RainRat on Github
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Dan Fandrich <dan@coneharvesters.com>
Closes: #13019
- Support I32 & I64 (eg: %I64d) for all Win32 builds.
Prior to this change mprintf support for the I format prefix, which is a
Microsoft extension, was dependent on the compiler used.
When Borland compiler support was removed in fd7ef00f the prefix was
then no longer supported for that compiler; however since it's still
possible to build with Borland I'm restoring support for the prefix in
this way.
Reported-by: Paweł Witas
Fixes https://github.com/curl/curl/issues/12944
Closes https://github.com/curl/curl/pull/12950
In a test case using lots of snprintf() calls using many commonly used
%-codes per call, this version is around 30% faster than previous
version.
It also fixes the #12561 bug which made it not behave correctly when
given unknown %-sequences. Fixing that flaw required a different take on
the problem, which resulted in the new two-arrays model.
lib557: extended - Verify the #12561 fix and test more printf features
unit1398: fix test: It used a <num>$ only for one argument, which is not
supported.
Fixes#12561Closes#12563
- enable `-Wsign-conversion` warnings, but also setting them to not
raise errors.
- fix `-Warith-conversion` warnings seen in CI.
These are triggered by `-Wsign-converion` and causing errors unless
explicitly silenced. It makes more sense to fix them, there just a few
of them.
- fix some `-Wsign-conversion` warnings.
- hide `-Wsign-conversion` warnings with a `#pragma`.
- add macro `CURL_WARN_SIGN_CONVERSION` to unhide them on a per-build
basis.
- update a CI job to unhide them with the above macro:
https://github.com/curl/curl/actions/workflows/linux.yml -> OpenSSL -O3
Closes#12492
A new error code to be used when an internal field grows too large, like
when a dynbuf reaches its maximum. Previously it would return
CURLE_OUT_OF_MEMORY for this, which is highly misleading.
Ref: #12268Closes#12269
https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
as of 2023-11-29 [1].
Enable new recommended warnings (except `-Wsign-conversion`):
- enable `-Wformat=2` for clang (in both cmake and autotools).
- add `CURL_PRINTF()` internal attribute and mark functions accepting
printf arguments with it. This is a copy of existing
`CURL_TEMP_PRINTF()` but using `__printf__` to make it compatible
with redefinting the `printf` symbol:
https://gcc.gnu.org/onlinedocs/gcc-3.0.4/gcc_5.html#SEC94
- fix `CURL_PRINTF()` and existing `CURL_TEMP_PRINTF()` for
mingw-w64 and enable it on this platform.
- enable `-Wimplicit-fallthrough`.
- enable `-Wtrampolines`.
- add `-Wsign-conversion` commented with a FIXME.
- cmake: enable `-pedantic-errors` the way we do it with autotools.
Follow-up to d5c0351055#2747
- lib/curl_trc.h: use `CURL_FORMAT()`, this also fixes it to enable format
checks. Previously it was always disabled due to the internal `printf`
macro.
Fix them:
- fix bug where an `set_ipv6_v6only()` call was missed in builds with
`--disable-verbose` / `CURL_DISABLE_VERBOSE_STRINGS=ON`.
- add internal `FALLTHROUGH()` macro.
- replace obsolete fall-through comments with `FALLTHROUGH()`.
- fix fallthrough markups: Delete redundant ones (showing up as
warnings in most cases). Add missing ones. Fix indentation.
- silence `-Wformat-nonliteral` warnings with llvm/clang.
- fix one `-Wformat-nonliteral` warning.
- fix new `-Wformat` and `-Wformat-security` warnings.
- fix `CURL_FORMAT_SOCKET_T` value for mingw-w64. Also move its
definition to `lib/curl_setup.h` allowing use in `tests/server`.
- lib: fix two wrongly passed string arguments in log outputs.
Co-authored-by: Jay Satiro
- fix new `-Wformat` warnings on mingw-w64.
[1] 56c0fde389/docs/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C%2B%2B.mdCloses#12489
Enable more picky compiler warnings. I've found these options in the
nghttp3 project when implementing the CMake quick picky warning
functionality for it [1].
`-Wunused-macros` was too noisy to keep around, but fixed a few issues
it revealed while testing.
- autotools: reflect the more precisely-versioned clang warnings.
Follow-up to 033f8e2a08#12324
- autotools: sync between clang and gcc the way we set `no-multichar`.
- autotools: avoid setting `-Wstrict-aliasing=3` twice.
- autotools: disable `-Wmissing-noreturn` for MSYS gcc targets [2].
It triggers in libtool-generated stub code.
- lib/timeval: delete a redundant `!MSDOS` guard from a `WIN32` branch.
- lib/curl_setup.h: delete duplicate declaration for `fileno`.
Added in initial commit ae1912cb0d
(1999-12-29). This suggests this may not be needed anymore, but if
it does, we may restore this for those specific (non-Windows) systems.
- lib: delete unused macro `FTP_BUFFER_ALLOCSIZE` since
c1d6fe2aaa.
- lib: delete unused macro `isxdigit_ascii` since
f65f750742.
- lib/mqtt: delete unused macro `MQTT_HEADER_LEN`.
- lib/multi: delete unused macro `SH_READ`/`SH_WRITE`.
- lib/hostip: add `noreturn` function attribute via new `CURL_NORETURN`
macro.
- lib/mprintf: delete duplicate declaration for `Curl_dyn_vprintf`.
- lib/rand: fix `-Wunreachable-code` and related fallouts [3].
- lib/setopt: fix `-Wunreachable-code-break`.
- lib/system_win32 and lib/timeval: fix double declarations for
`Curl_freq` and `Curl_isVistaOrGreater` in CMake UNITY mode [4].
- lib/warnless: fix double declarations in CMake UNITY mode [5].
This was due to force-disabling the header guard of `warnless.h` to
to reapply it to source code coming after `warnless.c` in UNITY
builds. This reapplied declarations too, causing the warnings.
Solved by adding a header guard for the lines that actually need
to be reapplied.
- lib/vauth/digest: fix `-Wunreachable-code-break` [6].
- lib/vssh/libssh2: fix `-Wunreachable-code-break` and delete redundant
block.
- lib/vtls/sectransp: fix `-Wunreachable-code-break` [7].
- lib/vtls/sectransp: suppress `-Wunreachable-code`.
Detected in `else` branches of dynamic feature checks, with results
known at compile-time, e.g.
```c
if(SecCertificateCopySubjectSummary) /* -> true */
```
Likely fixable as a separate micro-project, but given SecureTransport
is deprecated anyway, let's just silence these locally.
- src/tool_help: delete duplicate declaration for `helptext`.
- src/tool_xattr: fix `-Wunreachable-code`.
- tests: delete duplicate declaration for `unitfail` [8].
- tests: delete duplicate declaration for `strncasecompare`.
- tests/libtest: delete duplicate declaration for `gethostname`.
Originally added in 687df5c8c3
(2010-08-02).
Got complicated later: c49e9683b8
If there are still systems around with warnings, we may restore the
prototype, but limited for those systems.
- tests/lib2305: delete duplicate declaration for
`libtest_debug_config`.
- tests/h2-download: fix `-Wunreachable-code-break`.
[1] a70edb08e9/cmake/PickyWarningsC.cmake
[2] https://ci.appveyor.com/project/curlorg/curl/builds/48553586/job/3qkgjauiqla5fj45?fullLog=true#L1675
[3] https://github.com/curl/curl/actions/runs/6880886309/job/18716044703?pr=12331#step:7:72https://github.com/curl/curl/actions/runs/6883016087/job/18722707368?pr=12331#step:7:109
[4] https://ci.appveyor.com/project/curlorg/curl/builds/48555101/job/9g15qkrriklpf1ut#L204
[5] https://ci.appveyor.com/project/curlorg/curl/builds/48555101/job/9g15qkrriklpf1ut#L218
[6] https://github.com/curl/curl/actions/runs/6880886309/job/18716042927?pr=12331#step:7:290
[7] https://github.com/curl/curl/actions/runs/6891484996/job/18746659406?pr=12331#step:9:1193
[8] https://github.com/curl/curl/actions/runs/6882803986/job/18722082562?pr=12331#step:33:1870Closes#12331
- Pelles C: Unclear status, failed to obtain a fresh copy a few months
ago. Possible website is HTTP-only. ~10 years ago I left this compiler
dealing with crashes and other issues with no response on the forum
for years. It has seen some activity in curl back in 2021.
- LCC: Last stable release in September 2002.
- Salford C: Misses winsock2 support, possibly abandoned? Last mentioned
in 2006.
- Borland C++: We dropped Borland C++ support in 2018.
- MS Visual C++ 6.0: Released in 1998. curl already requires VS 2010
(or possibly 2008) as a minimum.
Closes#12222
Out of 415 labels throughout the code base, 86 of those labels were
not at the start of the line. Which means labels always at the start of
the line is the favoured style overall with 329 instances.
Out of the 86 labels not at the start of the line:
* 75 were indented with the same indentation level of the following line
* 8 were indented with exactly one space
* 2 were indented with one fewer indentation level then the following
line
* 1 was indented with the indentation level of the following line minus
three space (probably unintentional)
Co-Authored-By: Viktor Szakats
Closes#11134
The open paren check wants to warn for spaces before open parenthesis
for if/while/for but also for any function call. In order to avoid
catching function pointer declarations, the logic allows a space if the
first character after the open parenthesis is an asterisk.
I also spotted what we did not include "switch" in the check but we should.
This check is a little lame, but we reduce this problem by not allowing
that space for if/while/for/switch.
Reported-by: Emanuele Torre
Closes#11044
- they are mostly pointless in all major jurisdictions
- many big corporations and projects already don't use them
- saves us from pointless churn
- git keeps history for us
- the year range is kept in COPYING
checksrc is updated to allow non-year using copyright statements
Closes#10205
An input like "%.*1$.9999d" would first use the precision taken as an
argument *and* then the precision specified in the string, which is
confusing and wrong. pass1 will now instead return error on this double
use.
Adjusted unit test 1398 to verify
Reported-by: Peter Goodman
Closes#9754
This is the single place in libcurl code where it uses the "native"
s(n)printf() function. Used for writing floats. The use has been
reviewed and vetted and uses a HUGE target buffer, but switching to
snprintf() still makes this safer and removes build-time warnings.
Reported-by: Philip Heiduck
Fixes#9569Closes#9570
This function no longer returns a negative value if the formatting
string is bad since the return value would sometimes be propagated as a
return code from the mprintf* functions and they are documented to
return the length of the output. Which cannot be negative.
Fixes#9149Closes#9151
Reported-by: yiyuaner on github
Add licensing and copyright information for all files in this repository. This
either happens in the file itself as a comment header or in the file
`.reuse/dep5`.
This commit also adds a Github workflow to check pull requests and adapts
copyright.pl to the changes.
Closes#8869
Turns out that in dprintf_formatf we did a strlen on empty strings, a
bit strange is how common this actually is, 24 alone when doing a simple
GET from https://curl.seCloses#8427
Follow-up to bbf8cae44d
We removed support for the watcom builds files back in September
2020. This removes all remaining watcom references and ifdefs.
Closes#8287
... by introducing a printf() function that appends directly into a
dynbuf: Curl_dyn_vprintf(). This avoids the mandatory extra malloc so if
the buffer is already big enough it can just printf directly into it.
Since this less-malloc version requires tthe use of a library internal
printf function, we only provide this version when building libcurl and
not for the dynbuf code that is used when building the curl tool.
Closes#5998
Stack overflows can occur with precisions for integers and floats.
Proof of concepts:
- curl_mprintf("%d, %.*1$d", 500, 1);
- curl_mprintf("%d, %+0500.*1$f", 500, 1);
Ideally, compile with -fsanitize=address which makes this undefined
behavior a bit more defined for debug purposes.
The format strings are valid. The overflows occur due to invalid
arguments. If these arguments are variables with contents controlled
by an attacker, the function's stack can be corrupted.
Also see CVE-2016-9586 which partially fixed the float aspect.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Closes https://github.com/curl/curl/pull/5722
Verify that specified parameters are in range. If parameters are too
large, fail early on and avoid out of boundary accesses.
Also do not read behind boundaries of illegal format strings.
These are defensive measures since it is expected that format strings
are well-formed. Format strings should not be modifiable by user
input due to possible generic format string attacks.
Closes https://github.com/curl/curl/pull/5722
- Stick to a single unified way to use structs
- Make checksrc complain on 'typedef struct {'
- Allow them in tests, public headers and examples
- Let MD4_CTX, MD5_CTX, and SHA256_CTX typedefs remain as they actually
typedef different types/structs depending on build conditions.
Closes#5338
A common set of functions instead of many separate implementations for
creating buffers that can grow when appending data to them. Existing
functionality has been ported over.
In my early basic testing, the total number of allocations seem at
roughly the same amount as before, possibly a few less.
See docs/DYNBUF.md for a description of the API.
Closes#5300
- Disable warning C4127 "conditional expression is constant" globally
in curl_setup.h for when building with Microsoft's compiler.
This mainly affects building with the Visual Studio project files found
in the projects dir.
Prior to this change the cmake and winbuild build systems already
disabled 4127 globally for when building with Microsoft's compiler.
Also, 4127 was already disabled for all build systems in the limited
circumstance of the WHILE_FALSE macro which disabled the warning
specifically for while(0). This commit removes the WHILE_FALSE macro and
all other cruft in favor of disabling globally in curl_setup.
Background:
We have various macros that cause 0 or 1 to be evaluated, which would
cause warning C4127 in Visual Studio. For example this causes it:
#define Curl_resolver_asynch() 1
Full behavior is not clearly defined and inconsistent across versions.
However it is documented that since VS 2015 Update 3 Microsoft has
addressed this somewhat but not entirely, not warning on while(true) for
example.
Prior to this change some C4127 warnings occurred when I built with
Visual Studio using the generated projects in the projects dir.
Closes https://github.com/curl/curl/pull/4658
The overflow has no real world impact.
Just avoid it for "best practice".
Code change suggested by "The Infinnovation Team" and Daniel Stenberg.
Closes#3184
Fix the following warnings when building the tests by using the correct
types:
cast from 'const char *' to 'void *' drops const qualifier
[-Wcast-qual]
implicit conversion changes signedness [-Wsign-conversion]
This fixes the following clang warnings:
macro is not used [-Wunused-macros]
will never be executed [-Wunreachable-code]
Closes https://github.com/curl/curl/pull/1448
Visual C++ complained:
warning C4267: '=': conversion from 'size_t' to 'long', possible loss of data
warning C4701: potentially uninitialized local variable 'path' used
