Skip to content

Commit

Permalink
libcurl-thread.3: Clarify CURLOPT_NOSIGNAL takes long value 1L
Browse files Browse the repository at this point in the history
  • Loading branch information
@jay
jay committed Jul 29, 2015
1 parent b656715 commit 4673094
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions docs/libcurl/libcurl-thread.3
View file
Expand Up @@ -84,9 +84,9 @@ You may need to provide one or two functions to allow it to function properly:
\fBSignals.\fP Signals are used for timing out name resolves (during DNS
lookup) - when built without using either the c-ares or threaded resolver
backends. When using multiple threads you should set the
\fICURLOPT_NOSIGNAL(3)\fP option to 1 for all handles. Everything will or might
work fine except that timeouts are not honored during the DNS lookup - which
you can work around by building libcurl with c-ares support. c-ares is a
\fICURLOPT_NOSIGNAL(3)\fP option to 1L for all handles. Everything will or
might work fine except that timeouts are not honored during the DNS lookup -
which you can work around by building libcurl with c-ares support. c-ares is a
library that provides asynchronous name resolves. On some platforms, libcurl
simply will not function properly multi-threaded unless this option is set.

Expand Down

7 comments on commit 4673094

@jay
Copy link
Member Author

@jay jay commented on 4673094 Jul 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bagder I've since noticed the doc for CURLOPT_NOSIGNAL says use 1 as well instead of what I would expect 1L. I was under the impression after some investigating I did in #346 that if the value type is long it must be explicit because that could be an issue for some platforms with va_args. In Windows long and int are the same width and there's no difference but I'm curious about the details surrounding this behavior for other platforms. I'm now noticing places in the code where integers are used, see the protocol flags for example, and I did something similar myself when I added a flag to ssl options.

@bagder
Copy link
Member

@bagder bagder commented on 4673094 Jul 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, all the numerical varargs take longs and we should make sure to mention long or use L suffixes in the documentation.

@jay
Copy link
Member Author

@jay jay commented on 4673094 Jul 30, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bagder can you clarify, is it acceptable for options that only have a single arg value to pass an int instead of a long? I know it can be a problem for options that take multiple arg values. As I said I can see in the code that the flags used as values for some options that specify long values are basically ints (ie 1<<1) which would imply that it's acceptable.

My concern is the user may not pass the correct type. Although you could argue user error here I'd say it's generally assumed that when you pass a value 1 to functions that take a long that value becomes a long instead of staying as an integer as is the case here due to va args. For example take CURLOPT_MAXREDIRS which says in its description:

Set it to -1 for an infinite number of redirects.

I think without some sort of epxlicit notice in that option stating that the type must be long that you'd get users that do curl_easy_setopt(curl, CURLOPT_MAXREDIRS, -1); which is incorrect.

I experimented with explicitly tacking on the long literal-suffix L to all the numbers used to specify value in the options that take long. You can see that at jay@ca7c8da. The more I look at it the less I like it.

I wonder though, assuming the type must be long and not int or badness will happen, that maybe something different should be done, like for each option have a new section IMPORTANT and say something like

It is important that you specify the data types exactly as specified. If the data type is long, you must not pass an int. For more detail please read the overview of xyz.

@bagder
Copy link
Member

@bagder bagder commented on 4673094 Jul 30, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it acceptable for options that only have a single arg value to pass an int instead of a long?

On non-windows 64bit systems that will generally lead to badness, exactly as you say. I've tried to write that a long must be passed in all descriptions of numerical vararg options but I'm sure it can be emphasized better and as you say, showing just "-1" is fairly misleading or downright wrong. Although "-1" is of course not exactly stating if it is long or int.

As definitions are just numbers I've tried to not imply that they are integers or longs so that implementations can choose to use them however they want, it is only passing them as arguments to libcurl vararg functions that need to have them as longs.

(as a side-note I could mention that I believe this int/long confusion is among the worst design decisions I've tricked us into having...)

@jay
Copy link
Member Author

@jay jay commented on 4673094 Aug 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about the new section idea? It would mean adding something like this to every single option:

.SH IMPORTANT - TYPE ADHERENCE
Option parameters passed to curl_easy_setopt \fBmust\fP be the type specified.
For example, if the type is a long you \fBmust\fP pass a long and not an int or
any other type. If you are passing an integer literal and the parameter type is
not int you \fBmust\fP cast or append the proper type suffix. For example, to
pass a 0 of type long you would pass 0L.

@bagder
Copy link
Member

@bagder bagder commented on 4673094 Aug 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to make it specific for long options, which then can make us use a more direct language. We rarely have a problem with the types of the other options. Or the curl_off_t options could get something similar to the long one but adjusted to that type?

@jay
Copy link
Member Author

@jay jay commented on 4673094 Aug 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, what do you think of this

.SH TYPE ADHERENCE
\fBIMPORTANT:\fP This option takes a parameter of type long and \fByou must
pass a long and not an int or any other type.\fP Cast or append the proper type
suffix. Example: to pass a 0 of type long you must pass 0L and not 0.  Example:
to pass int x you must pass (long)x and not x.

Please sign in to comment.