Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os400: Definition not found for symbol 'Curl_vsetopt' #3833

Closed
jonrumsey opened this issue May 3, 2019 · 8 comments
Closed

os400: Definition not found for symbol 'Curl_vsetopt' #3833

jonrumsey opened this issue May 3, 2019 · 8 comments

Comments

@jonrumsey
Copy link
Contributor

I did this

Attempted to build libcurl 7.64.1 on os400 V7R2M0:
CPD5D02: Definition not found for symbol 'Curl_vsetopt'.
CPD5D19: 1 warnings were issued from binder language compilation.
CPF5D05: Service program LIBCURL not created.

This looks like it may have been as a result of converting Curl_vsetopt() to a static function vsetopt() in lib/setopt.c, however there is still a call to Curl_vsetopt() in packages/OS400/ccsidcurl.c.

I expected the following

A clean build

curl/libcurl version

7.64.1

operating system

IBM i V7R2M0

@bagder bagder added the build label May 3, 2019
@bagder
Copy link
Member

bagder commented May 3, 2019

Thanks, yes that seems to be accurate. How about changing that function call to something this:

--- a/packages/OS400/ccsidcurl.c
+++ b/packages/OS400/ccsidcurl.c
@@ -1303,13 +1303,16 @@ curl_easy_setopt_ccsid(CURL * curl, CURLoption tag, ...)
     data->set.str[STRING_COPYPOSTFIELDS] = s;   /* Give to library. */
     break;
 
   case CURLOPT_ERRORBUFFER:                     /* This is an output buffer. */
   default:
-    result = Curl_vsetopt(data, tag, arg);
+  {
+    long val = va_arg(arg, long);
+    result = curl_easy_setopt(curl, tag, val);
     break;
-    }
+  }
+  }
 
   va_end(arg);
   return result;
 }

@jonrumsey
Copy link
Contributor Author

Thanks for the quick response, yes, your proposed change builds fine and I now have a clean 7.64.1 build.

@bagder
Copy link
Member

bagder commented May 3, 2019

Thanks for confirming!

@bagder bagder closed this as completed in bccf1dc May 3, 2019
@monnerat
Copy link
Contributor

This fix may build, but it surely won't run properly: while longs are 32-bits, pointers are 128-bits !
In addition, alignment is important and pointers must be handled as such or else they would lose their addressing capabilities.
I already faced this (3b548ff) and use of Curl_vsetopt() is the easiest way to avoid locally dispatching on tag argument CURLOPTTYPE_* to determine the proper type and proper calling sequence.

@monnerat monnerat reopened this May 23, 2019
@jonrumsey
Copy link
Contributor Author

I suppose one way to fix this is to pass a pointer to curl_easy_setopt using va_arg(arg, char *) instead of the long value. curl_easy_setopt_ccsid() should ensure that callers are specifying a suitable tag with additional switch statements, the default case should return CURLE_UNKNOWN_OPTION.

I will try this approach against 7.65.1.

@monnerat
Copy link
Contributor

There are 2 ways to fix it:

  1. Reinstate Curl_vsetopt() as it was before. This is the easiest, more efficient and elegant change. Since Daniel already made static this function twice, I was expecting a comment to go/nogo this way.
  2. Dispatch on the CURLOPTTYPE_* part of the option, branching to an appropriate call to acurl_easy_setopt() call for the given parameter type. Feasible but has no advantage on the previous solution.

@bagder
Copy link
Member

bagder commented Jun 14, 2019

Reinstate Curl_vsetopt() as it was before

Reverting the lib/setopt.c part of 05b100a should probably be enough. I wasn't aware of this use of the function when I made it static - and I would propose that we add a comment about it this time so that we can avoid doing this exact same dance again in a future! 😄

@monnerat
Copy link
Contributor

Reinstate Curl_vsetopt() as it was before

I can do it... with a comment :-)

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants