kubernetes-client / c

Official C client library for Kubernetes
Apache License 2.0
146 stars 46 forks source link

sslConfig_create didn't return a pointer #22

Closed clearday4 closed 4 years ago

clearday4 commented 4 years ago

sslConfig_create() is not void function but not return a sslConfig pointer. Need to add return sslConfig.

ityuhui commented 4 years ago

Hi @clearday4

Thank you for your finding ! You're right, the function should return sslConfig

sslConfig_t *sslConfig_create(const char *clientCertFile, const char *clientKeyFile, const char *CACertFile, int insecureSkipTlsVerify) {
    sslConfig_t *sslConfig = calloc(1, sizeof(sslConfig_t));
    if ( clientCertFile ) {
        sslConfig->clientCertFile = strdup(clientCertFile);
    }
    if ( clientKeyFile ) {
        sslConfig->clientKeyFile = strdup(clientKeyFile);
    }
    if ( CACertFile ) {
        sslConfig->CACertFile = strdup(CACertFile);
    }
    sslConfig->insecureSkipTlsVerify = insecureSkipTlsVerify;
    <== Yes,sslConfig shoud be returned here. It's my fault.
}

I have not found this issue before because the sslConfig is returned to caller actually. Maybe the compiler/gcc handles this defect and returns the sslConfig automatically. So this defect has no impact on gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04).

But anyway, this defect should be fixed. I will handle it some times later. If anyone has interest, the fix should be done in the project OpenAPI-Generator/c

Thank you.

brendandburns commented 4 years ago

I suspect that this is because of x86 conventions and the pointer being stored in the same register as the expected return value:

https://stackoverflow.com/questions/7280877/why-and-how-does-gcc-compile-a-function-with-a-missing-return-statement

but we should definitely fix this.

brendandburns commented 4 years ago

I filed: https://github.com/OpenAPITools/openapi-generator/pull/7217

in the generator to fix this.

brendandburns commented 4 years ago

The code generator patch is merged (though there are other issues that were detected)

We should fix the other issues, and then regenerate.