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

Avoid magic allocation of SSL backend specific data #2119

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@dscho
Contributor

dscho commented Nov 28, 2017

Originally, my idea was to allocate the two structures (or more
precisely, the connectdata structure and the four SSL backend-specific
strucutres required for ssl[0..1] and proxy_ssl[0..1]) in one go, so
that they all could be free()d together.

However, getting the alignment right is tricky. Too tricky.

So let's just bite the bullet and allocate the SSL backend-specific
data separately.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Nov 28, 2017

Contributor

Let's see what the CI says.

Contributor

dscho commented Nov 28, 2017

Let's see what the CI says.

Avoid magic allocation of SSL backend specific data
Originally, my idea was to allocate the two structures (or more
precisely, the connectdata structure and the four SSL backend-specific
strucutres required for ssl[0..1] and proxy_ssl[0..1]) in one go, so
that they all could be free()d together.

However, getting the alignment right is tricky. Too tricky.

So let's just bite the bullet and allocate the SSL backend-specific
data separately.

As a consequence, we now have to be very careful to release the memory
allocated for the SSL backend-specific data whenever we release any
connectdata.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Nov 28, 2017

Contributor

Let's see what the CI says.

It said that I indeed had forgotten a few places where struct connectdata is released. I tried to find all remaining places, and I think I found them: when allocate_conn() fails, we already released master_buffer and localdev, now also ssl_extra, and when create_conn() releases conn after calling reuse_conn(), we now also release ssl_extra.

CI, here you go again.

Contributor

dscho commented Nov 28, 2017

Let's see what the CI says.

It said that I indeed had forgotten a few places where struct connectdata is released. I tried to find all remaining places, and I think I found them: when allocate_conn() fails, we already released master_buffer and localdev, now also ssl_extra, and when create_conn() releases conn after calling reuse_conn(), we now also release ssl_extra.

CI, here you go again.

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Nov 28, 2017

Contributor

CI, here you go again.

There you go. All green again.

@bagder as agreed before, no rush on this PR.

Contributor

dscho commented Nov 28, 2017

CI, here you go again.

There you go. All green again.

@bagder as agreed before, no rush on this PR.

@bagder

bagder approved these changes Nov 29, 2017

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Dec 1, 2017

Member

Thanks!

Member

bagder commented Dec 1, 2017

Thanks!

@bagder bagder closed this in 9194a99 Dec 1, 2017

@dscho dscho deleted the dscho:alloc-ssl-backends-separately branch Dec 1, 2017

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Dec 1, 2017

Contributor

Let's hope that I did not introduce yet another regression! ;-)

Contributor

dscho commented Dec 1, 2017

Let's hope that I did not introduce yet another regression! ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment