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.

Sign up for GitHub

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

Jump to bottom

FIX strncpy warning in apps/cmp.c. #12889

Closed
wants to merge 1 commit into from
Closed

FIX strncpy warning in apps/cmp.c. #12889

wants to merge 1 commit into from

Conversation

xffbai
Copy link
Contributor

@xffbai xffbai commented Sep 16, 2020

bugfix: #12872

strncpy here has compiling warning of -Wstringop-truncation, change into BIO_snprintf as before.

Checklist
  • documentation is added or updated
  • tests are added or updated

@xffbai
FIX strncpy warning in apps/cmp.c.
9cf0623 
bugfix: openssl#12872

strncpy here has compiling warning of -Wstringop-truncation, change
into BIO_snprintf as before.

Change-Id: I362872c4ad328cadd4c7a5a5da3165655fa26c0d
@xffbai
Copy link
Contributor Author

xffbai commented Sep 16, 2020 •
edited
Loading

@DDvO I think it is related to(6e477a6), could you please have a look at this?

DDvO
DDvO approved these changes Sep 16, 2020
View reviewed changes
Copy link
Contributor

@DDvO DDvO left a comment

Choose a reason for hiding this comment

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

LGTM
(assuming that BIO_snprintf() always terminates the string within the buffer with a \0, even on error).

@DDvO
Copy link
Contributor

DDvO commented Sep 16, 2020

Thanks @xffbai for taking care of this.

@paulidale paulidale added approval: done This pull request has the required number of approvals branch: master Merge to master branch and removed approval: otc review pending labels Sep 16, 2020
@DDvO
Copy link
Contributor

DDvO commented Sep 16, 2020

This also fixes Coverity CID 1466707:

 Memory - illegal accesses  (BUFFER_SIZE_WARNING)
    Calling "strncpy" with a maximum size argument of 32 bytes on destination array "server_port" of size 32 bytes might leave the destination string unterminated.

@kroeckx kroeckx added this to the 3.0.0 milestone Sep 16, 2020
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@t8m t8m added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Sep 17, 2020
openssl-machine pushed a commit that referenced this pull request Sep 17, 2020
@xffbai @DDvO
FIX strncpy warning in apps/cmp.c.
ebcae87 
bugfix: #12872

strncpy here has compiling warning of -Wstringop-truncation, change
into BIO_snprintf as before.

Change-Id: I362872c4ad328cadd4c7a5a5da3165655fa26c0d

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from #12889)
@DDvO
Copy link
Contributor

DDvO commented Sep 17, 2020

Merged - thanks

@DDvO DDvO closed this Sep 17, 2020
@kroeckx kroeckx mentioned this pull request Oct 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DDvO DDvO DDvO approved these changes

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@xffbai @DDvO @openssl-machine @paulidale @kroeckx @t8m