Bugs item #3077599, was opened at 2010-09-28 23:41
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3077599&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: portability problem
Status: Open
>Resolution: Invalid
Priority: 5
Private: No
Submitted By: Jeff A. Marr (jamarr81)
Assigned to: Daniel Stenberg (bagder)
Summary: Add curl_easy_setoptv

Initial Comment:
I am writing a small c++ wrapper for curl and came across an issue with curl_easy_setopt. There is no va_list alternative version of this function making it impossible to encapsulate; see: http://c-faq.com/varargs/handoff.html. To fix this we simply create a va_list version of this function: curl_easy_setoptv. I have attached a very small patch to fix this issue.

----------------------------------------------------------------------

>Comment By: Daniel Stenberg (bagder)
Date: 2010-09-29 15:51

Message:
curl_easy_setopt() will remain accepting three parameters, out of which the
third can be of different kinds. Thus it is easy enough to write a wrapper
for it. I don't think a new function for the varargs approach is needed in
libcurl.

----------------------------------------------------------------------

Comment By: Dan Fandrich (dfandrich)
Date: 2010-09-29 03:04

Message:
I believe the only reason curl_easy_setopt uses a variable argument
parameter is because C doesn't allow overloaded functions. If you stop
thinking it as a varargs function but instead a function taking a single
argument, then your adapter can be written:

template <typename T> CURLcode CURL::setOption(CURLoption option, T arg)
{
  return curl_easy_setopt(option, arg);
}

That's 5 lines fewer than your example :)

Of course, this breaks if libcurl ever starts using curl_easy_setopt
options with more than one parameter, in which case adding a va_list
alternative seems necessary.

----------------------------------------------------------------------

Comment By: Jeff A. Marr (jamarr81)
Date: 2010-09-29 02:16

Message:
I also think it is worth noting that all standard vararg functions have
va_list variants to allow users to easily write adapters for them and that
writing these variants is common, and in general expected, practice.

----------------------------------------------------------------------

Comment By: Jeff A. Marr (jamarr81)
Date: 2010-09-29 02:08

Message:
bagder, you are correct that it is not impossible. As noted in main.c, you
can manually filter and dereference the va_list as necessary. And in cases
where additional filtering is necessary, this would have to be done to some
extent anyway. However, for a simple 1-1 adapter method or any adapter that
only needs to filter <MAX options, this is overkill. Take for example, with
my patch, a simple 1-1 adapter requires only 6 lines of code (c++):
[code]
CURLcode Curl::setOption(CURLoption option, ...)
{
        va_list args;
        va_start(args, option);
        CURLcode ret = curl_easy_setoptv(handle, option, args);
        va_end(args);
        return ret;
}
[/code]

Even if the existing work-around is easily understood, the influx of code
is unacceptable. More code introduces the potential for more bugs. If there
is a simpler, more elegant, more efficient solution then why avoid it?

dfandrich, you are reiterating the same idea. In either case, the user has
to take additional and unnecessary measures to work around an issue with a
very simple solution. Just because something is possible does not make it
plausible. In fact, this argument only strengthen the reason for applying
this patch: usability.

----------------------------------------------------------------------

Comment By: Dan Fandrich (dfandrich)
Date: 2010-09-29 00:11

Message:
Every curl_easy_setopt option except one takes a single argument in the
"variable" argument section (the outlier takes no arguments at all--any
argument will be ignored). So there isn't a problem in encapsulating the
function due to the number of parameters, only the parameter's type, and
that's easy to overcome with polymorphism.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2010-09-28 23:52

Message:
It is not impossible at all. In fact it's even fairly easy. See the
src/main.c:_my_setopt() function for how we do it within the curl tool
source code.

Your function does make it easier but with the fancy type checks we have
for curl_easy_setopt() now and the fact that we can easily work around this
little flaw, I'm not at all convinced we should add this new function you
suggest.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3077599&group_id=976
Received on 2010-09-29