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

Comments

@jonrumsey
Copy link

commented May 3, 2019

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

commented May 3, 2019

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

@bagder

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Thanks for confirming!

@bagder bagder closed this in bccf1dc May 3, 2019
@monnerat

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

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
@bagder bagder added the help wanted label Jun 10, 2019
@jonrumsey

This comment has been minimized.

Copy link
Author

commented Jun 14, 2019

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

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2019

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2019

Reinstate Curl_vsetopt() as it was before

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

@monnerat monnerat closed this in ef8d98b Jun 15, 2019
@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.
Projects
None yet
3 participants
You can’t perform that action at this time.