On Tue, 10 Dec 2019, Daniel Stenberg via curl-library wrote:
> On Mon, 9 Dec 2019, Jeff Mears via curl-library wrote:
>
> > curl_easy_setopt and curl_multi_setopt don't currently have a C++
> > implementation that enforces type checking. In C, it uses GCC
> > extensions to do this type checking.
> Right, because there's no native "real" C way to do it.
It could be done using C11's _Generic, but it would certainly be awkward.
The hard part is supporting the ability to have the option enum be a runtime
value. If the option enum had to be a compile-time value, generating a
compiler error for using the wrong type would actually be straightforward with
C11's _Generic. Supporting runtime values for the option enum as runtime
asserts and compile-time values as compiler errors is impossible in straight C,
but possible with GCC's __builtin_constant_p as libcurl's existing code does.
> > curl_easy_setopt(easy, CURLOPT_NOSIGNAL, 1);
> >
> > In C, this is broken.
>
> Yes; In C on platforms where long and int are sized differently...
It's actually undefined behavior even if they're the same size, but I've never
seen a compiler for which this would fail when int and long are physically the
same.
> I don't know what methods you speak of so I can't tell if they are completely
> equal in terms of readability, maintainability etc.
Here's one way it could be implemented that I came up with. However, it
requires that the option parameter be a compile-time value.
namespace curl_typecheck
{
template <typename T> struct setopt_type
{
setopt_type(int o) : option(o) { }
int option;
};
template <int Option> struct setopt_option;
// The list of options and their correct types is here.
template <> struct setopt_option<CURLOPT_NOSIGNAL> : setopt_type<long> { };
template <> struct setopt_option<CURLOPT_XFERINFOFUNCTION> :
setopt_type<curl_xferinfo_callback> { };
template <typename T, typename U>
inline int setopt_call(CURL *handle, setopt_type<T> opt, const U ¶m)
{
static_assert(std::is_same_v<T, U>(opt), "unexpected type for option");
return curl_easy_setopt(handle, opt.option, static_cast<T>(param));
}
}
#define curl_easy_setopt(handle, opt, ...)
(::curl_typecheck::setopt_call(handle,
::curl_typecheck::setopt_option<(opt)>(opt), __VA_ARGS__))
Note that by the rules of C and C++, the following is undefined behavior:
struct Meow { FILE *x; };
static Meow meow;
...
curl_easy_setopt(easy, CURLOPT_XFERINFODATA, &meow);
This is undefined behavior because by C's definition of va_arg, the type passed
to va_arg by curl_easy_setopt must match the actual type that was passed in.
curl_easy_setopt uses void * with va_arg:
case CURLOPT_PROGRESSDATA:
/*
* Custom client data to pass to the progress callback
*/
data->set.progress_client = va_arg(param, void *);
break;
void * and Meow * are different types, so that's undefined behavior. The only
exceptions to the va_arg rule are that for integer types signedness doesn't
matter and that void * is allowed to match char * or unsigned char * in both
directions.
Again, like the same-sized int vs. long case, I've never seen a compiler for
which this is a problem, because every compiler I know of has the same
representation for void * as other pointer types. This is also true of printf
%p, too. Too much code out there already uses these without casting to void *
first.
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
