RE: curl_easy_setopt type checking for C++

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 &param)
  {
    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
Received on 2019-12-10