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

Renaming "new" variable #1783

Merged
merged 5 commits into from Jun 29, 2018

Conversation

Projects
None yet
4 participants
@hirotakaster

hirotakaster commented Jun 25, 2018

Description

Based on issue #1782
Replace *new to *new_cert in ssl_append_key_cert function at ssl_tls.c.

Status

READY

Requires Backporting

NO

Migrations

NO

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

@hanno-arm hanno-arm self-requested a review Jun 25, 2018

@hanno-arm hanno-arm self-assigned this Jun 25, 2018

@hanno-arm

The actual change looks good to me. Could you please add an entry in the ChangeLog, including the platform on which the previous naming causes the build failure, the way you want to be credited, and the issue that this PR fixes? Thank you!

@RonEld

This comment has been minimized.

Contributor

RonEld commented Jun 25, 2018

@hirotakaster Thank you for your contribution!

In addition, unfortunately our policy is to not accept contributions, without a Contributor’s Licence Agreement (CLA) signed or authorised by yourself or your employer.
If this is a personal contribution, the easiest way to do this is if you create an mbed account and accept this click through agreement. Alternatively, you can find a slightly different agreement to sign here, which can be signed and returned to us, and is applicable if you don't want to create an mbed account or alternatively if this is a corporate contribution.
Thanks for your understanding and again, thanks for the contribution!

@hirotakaster

This comment has been minimized.

hirotakaster commented Jun 25, 2018

@RonEld
I accepted CLA now.
thank you for your kindly.

@RonEld

This comment has been minimized.

Contributor

RonEld commented Jun 25, 2018

@hirotakaster Thank you for accepting the CLA! Could you kindly confirm that this is your Mbed account?

@hirotakaster

This comment has been minimized.

hirotakaster commented Jun 25, 2018

@RonEld
Yes, this is my mbed account.

@RonEld RonEld added CLA valid and removed CLA requested labels Jun 25, 2018

niisato
ChangeLog Outdated

Bugfix
* Fix Renaming "new" variable #1783.
compile error(new variable) with arm-none-eabi-gcc(c++) on mbed TLS 2.7.0.

This comment has been minimized.

@RonEld

RonEld Jun 25, 2018

Contributor

I would change this sentence to:

Fix compilation error on c++, because of a variable named new. Found and fixed by Hirotaka Niisato in #1783 

Unless you want to credit yourself with your github name.

This comment has been minimized.

@hirotakaster

hirotakaster Jun 25, 2018

Thank you, I'm okay "Hirotaka Niisato" .

@RonEld RonEld added the needs: work label Jun 25, 2018

@hirotakaster

This comment has been minimized.

hirotakaster commented Jun 25, 2018

@hanno-arm
I add ChangeLog to this PR now.
Thank you for your kindness.

niisato
@hirotakaster

This comment has been minimized.

hirotakaster commented Jun 25, 2018

Thank you @RonEld, I updated ChangeLog now according to your advice.

@RonEld

RonEld approved these changes Jun 25, 2018

@RonEld RonEld added needs: review and removed needs: work labels Jun 25, 2018

@hanno-arm

Minor change in the ChangeLog entry requested.

ChangeLog Outdated
= mbed TLS x.x.x branch released xxxx-xx-xx

Bugfix
* Fix compilation error on c++, because of a variable named new. Found and fixed by Hirotaka Niisato in #1783

This comment has been minimized.

@hanno-arm

hanno-arm Jun 25, 2018

Contributor

Please split this into multiple lines to avoid lines of more than 80 characters. Also, please add a full stop at the end of the second sentence, and capitalize C++.

This comment has been minimized.

@hirotakaster

hirotakaster Jun 25, 2018

@hanno-arm
Now I update the ChangeLog.
Thank you.

niisato added some commits Jun 25, 2018

niisato
niisato
@hanno-arm

Looks good to me. Thanks @hirotakaster for the quick turnaround!

@hanno-arm

This comment has been minimized.

Contributor

hanno-arm commented Jun 25, 2018

@sbutcher-arm Ready to be merged from our perspective.

@RonEld

RonEld approved these changes Jun 25, 2018

Approving also latest changes to ChangeLog

@sbutcher-arm

This comment has been minimized.

Collaborator

sbutcher-arm commented Jun 27, 2018

Needs backporting to mbedtls-2.7, although I think that can be done with this PR without problem.

@sbutcher-arm sbutcher-arm merged commit 164b9cd into ARMmbed:development Jun 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sbutcher-arm

This comment has been minimized.

Collaborator

sbutcher-arm commented Jun 29, 2018

Backported to mbedtls-2.7 with PR #1819 and to mbedtls-2.1 with PR #1820.

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