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

More zalloc: fix default initialisation of some allocated objects #997

Closed
wants to merge 2 commits into from

Conversation

FdaSilvaYY
Copy link
Contributor

@FdaSilvaYY FdaSilvaYY commented Apr 26, 2016

1st commit is intended to be back-ported.
So, I revert some of his changes in the second commit directly.

@@ -135,6 +135,7 @@ BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it)
ndef_aux->ndef_bio = sarg.ndef_bio;
ndef_aux->boundary = sarg.boundary;
ndef_aux->out = out;
ndef_aux->derbuf = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed because of the zalloc call.

@FdaSilvaYY FdaSilvaYY force-pushed the more-zalloc2 branch 2 times, most recently from d0ef38a to 5107efa Compare April 26, 2016 19:12
@FdaSilvaYY FdaSilvaYY changed the title More zalloc: fix default initialisation of allocted objects More zalloc: fix default initialisation of some allocated objects Apr 27, 2016
@FdaSilvaYY FdaSilvaYY force-pushed the more-zalloc2 branch 4 times, most recently from 6b9617c to 3e2f7b4 Compare April 29, 2016 16:57
@richsalz richsalz self-assigned this Apr 29, 2016
@richsalz
Copy link
Contributor

To make sure I get it: the first commit is for 1.0.2/1.0.1 and the first and second are for master?
+1

@FdaSilvaYY
Copy link
Contributor Author

FdaSilvaYY commented Apr 29, 2016

Yes, that's it.
The second commit is only for the master.
I'm preparing a PR for the backport too, don't worry.

if (b == NULL)
return 0;

b->peer = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

No. The null pointer isn't necessarily all 0 bits. See http://c-faq.com/null/runtime0.html

Copy link
Member

Choose a reason for hiding this comment

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

That being said, I may have missed this in early similar PRs...

Copy link
Contributor

Choose a reason for hiding this comment

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

No, we only run on platforms where NULL is all-bits-zero. See test/nptest.c

Copy link
Contributor

Choose a reason for hiding this comment

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

You missed test/nptest :)

Copy link
Member

Choose a reason for hiding this comment

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

Ah!

Ok then

@mattcaswell
Copy link
Member

Ping @levitte

@mattcaswell mattcaswell added this to the 1.1.0 milestone May 16, 2016
@FdaSilvaYY FdaSilvaYY force-pushed the more-zalloc2 branch 5 times, most recently from c1c31db to 244ce06 Compare May 19, 2016 06:11
@FdaSilvaYY FdaSilvaYY force-pushed the more-zalloc2 branch 2 times, most recently from e323d67 to d3e647a Compare May 21, 2016 08:05
@mattcaswell
Copy link
Member

@levitte is this good to go?

@levitte
Copy link
Member

levitte commented May 23, 2016

Yes, looks good to me

@mattcaswell
Copy link
Member

Sorry, @levitte - was that a plus one?

@levitte
Copy link
Member

levitte commented May 27, 2016

Yes, sorry for the confusion. +1

@mattcaswell
Copy link
Member

Adding review label for @levitte. @richsalz will you merge?

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label May 28, 2016
@richsalz
Copy link
Contributor

done.

@richsalz richsalz closed this May 29, 2016
levitte pushed a commit that referenced this pull request May 29, 2016
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #997)
levitte pushed a commit that referenced this pull request May 29, 2016
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #997)
@FdaSilvaYY FdaSilvaYY deleted the more-zalloc2 branch May 29, 2016 06:14
FdaSilvaYY added a commit to FdaSilvaYY/openssl that referenced this pull request May 29, 2016
levitte pushed a commit that referenced this pull request May 31, 2016
Backport of 8e89e85
From PR #1019 / #997

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #1019)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants