Skip to content

Conversation

@richsalz
Copy link
Contributor

Backport from #5141

@richsalz richsalz added branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL) 1.1.0 approval: review pending This pull request needs review by a committer labels Jan 23, 2018
@richsalz richsalz self-assigned this Jan 23, 2018
Copy link
Member

Choose a reason for hiding this comment

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

as a BN_ULONG ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i suppose. but all three releases have the same wording.

Copy link
Member

Choose a reason for hiding this comment

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

remove this line,

Backport from #5141
@richsalz
Copy link
Contributor Author

Updated commit pushed that addresses both comments.

@mattcaswell mattcaswell added this to the 1.1.0 milestone Jan 24, 2018

If a B<BIGNUM> is equal to 0xffffffffL it can be represented as an
unsigned long but this value is also returned on error.
B<BN_ULONG> should probably be a typedef.

Choose a reason for hiding this comment

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

I think the should probably be a typedef comment should go. By now some feature tests likely exist that test for ifdef BN_ULONG, so it is too late to make this a type instead. Also, when we add new types, applications that support older releases can test for macros more easily than for new types (instead they'd need to test against the OPENSSL_VERSION_NUMBER that adds the new type).

So all in all while may at some point introduce new types as non-macros, this one should probably stay as-is long-term.

bernd-edlinger
bernd-edlinger approved these changes Jan 24, 2018
levitte pushed a commit that referenced this pull request Jan 24, 2018
Backport from #5141

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #5151)
@richsalz
Copy link
Contributor Author

Merged, thanks!

levitte pushed a commit that referenced this pull request Jan 24, 2018
Backport from #5141

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #5151)

(cherry picked from commit 8b2124a)
@richsalz richsalz closed this Jan 24, 2018
@richsalz richsalz deleted the gh5141-for-110 branch January 24, 2018 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: review pending This pull request needs review by a committer branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants