Prevent runtime warning on OS400 when CURL_DISABLE_PROXY is not defined.

[jonrumsey]

"*** WARNING: curl_easy_setopt_ccsid() should be reworked ***" appears at runtime on OS400 when using curl_easy_setopt_ccsid() because the checks added in ccsidcurl.c to handle ASCII to EBCDIC string conversions does not correctly cater for CURL_DISABLE_PROXY being defined. When this is defined the integer value of STRING_LASTZEROTERMINATED is STRING_SASL_AUTHZID + 1, however when it is not defined the value is STRING_TEMP_URL + 1. The code that generates the runtime warning should factor this into its checks.

[bagder]

I don't really understand that check. The STRING_ enums are used for internal string storage, they're not strictly related to curl_easy_setopt and I don't see anything in that file use any STRING_ enum other than STRING_COPYPOSTFIELDS...

@monnerat, do you recall?

[monnerat]

@monnerat, do you recall?

Yes, very well, although it's more than 12 years old :-)
In initial OS400 code review https://curl.haxx.se/mail/lib-2007-08/0290.html, Dan Fandrich wrote:

+CURLcode
+curl_easy_setopt_ccsid(CURL * curl, CURLoption tag, ...)
+
+{
+ CURLcode result;
+ va_list arg;
+ struct SessionHandle * data;
+ char * s;
+ unsigned int ccsid;
+
+ data = (struct SessionHandle *) curl;
+ va_start(arg, tag);
+
+ switch (tag) {
+
+ case CURLOPT_CAINFO:
+ case CURLOPT_CAPATH:

Probably a good idea to put an assert on CURLOPT_LASTENTRY on entry to
this function to catch the case where a new option is added without
modifying this function accordingly.

I took this idea, but checked on the STRING_* definitions instead of CURLOPT_* to avoid catching false positives on new options that are not strings (not subject to CCSID conversion).

As the warning message says, I have reworked the function since then when needed. However, conditional definitions of the last STRING_* symbol has always be the source of troubles for the benefit of only 1 pointer in array storage, and I would have preferred it's been avoided!

The use of a STRING_* slot to store something that is not a curl_easy_setopt() argument is something completely new and I have to admit it fools the check.

[bagder]

conditional definitions of the last STRING_* symbol has always be the source of troubles for the benefit of only 1 pointer in array storage, and I would have preferred it's been avoided!

I wasn't aware of this issue, but I agree with you that we can certainly avoid those #ifdefs as they don't offer much gain anyway. And we could mention that in a comment above the enum so that we don't do the mistake again...

[monnerat]

Regarding the check itself: today I would be in favor of replacing it with an external check at build time. But this would require the use of advanced scriptng and there is no serious standard (i.e.: available in OS400 distribution) scripting tool on OS400 but sh/sed/grep/etc. In addition, this would have to be debugged/tested by someone else.

[jonrumsey]

Thank you both for the feedback and I agree it would be better to know at build time that the OS400 string conversion code may need updating. I'll look at reworking the change to;

• Remove the conditional directives in the dupstring enum in urldata.h (with appropriate comment)
• Remove the runtime checks/warnings from OS400 ccsidcurl.c
• Add a small C test program that will validate at OS400 build time whether the enums have expected values

[monnerat]

I'll look at reworking the change to...

Great!
Using a C program is a bit more complicated than a simple script, but is the wisest option in the absence of scripting language.
This program ought to be compiled and run from packages/OS400/make-lib.sh and independent of the ./tests directory (which is not processed anymore on OS400)..
Please write it your own way: take care of having a program you don't need to update each time there is a new option. Maybe by reading sources (ccsidcurl.c and others), possibly inserting "landmark comments" in these sources if needed.
If you need help/additional hints, please feel free to contact me.
Thank you in advance for your work on this.