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

urldata: remove values that are not used depending on compilation settings #13188

Closed
wants to merge 1 commit into from

Conversation

MAntoniak
Copy link
Contributor

Reduced size of dynamically_allocated_data structure.

Reduced number of stored values in enum dupstring and enum dupblob. This affects the reduced array placed in the UserDefined structure.

It is also possible to remove the STRING_SASL_AUTHZID and STRING_BEARER values from the dupstring. However, this requires a separate PR.

STRING_SSL_CRLFILE, /* crl file to check certificate */
STRING_SSL_ISSUERCERT, /* issuer cert file to check certificate */
STRING_SERVICE_NAME, /* Service name */
#ifndef CURL_DISABLE_PROXY
Copy link
Member

Choose a reason for hiding this comment

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

This is in direct contradiction to the comment above the enum which says:

This enumeration MUST not use conditional directives (#ifdefs)

But: the comment also refers to a (misspelled) source code file that was removed in 821d108, which makes me suspect maybe this comment does not apply anymore?

@monnerat can you fill in some blanks maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the comment is obsolete: the current test is now based on CURLOPT_*, not on STRING_* anymore.

Copy link
Member

Choose a reason for hiding this comment

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

@MAntoniak can you perhaps update that comment to reflect reality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand correctly just remove it?

Copy link
Member

Choose a reason for hiding this comment

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

Yes!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

…tings.

Reduced size of dynamically_allocated_data structure.

Reduced number of stored values in enum dupstring and enum dupblob. This affects the reduced array placed in the UserDefined structure.
@bagder bagder closed this in f46385d Apr 5, 2024
@bagder
Copy link
Member

bagder commented Apr 5, 2024

Thanks!

vszakats pushed a commit to vszakats/curl that referenced this pull request Apr 7, 2024
Reduced size of dynamically_allocated_data structure.

Reduced number of stored values in enum dupstring and enum dupblob. This
affects the reduced array placed in the UserDefined structure.

Closes curl#13188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants