Add a missing break in test/shlibloadtest.c #3661
Conversation
| @@ -117,6 +117,7 @@ static int test_lib(void) | |||
| if (!TEST_true(shlib_load(path_crypto, &cryptolib)) | |||
| || !TEST_true(shlib_load(path_ssl, &ssllib))) | |||
| goto end; | |||
| break; | |||
There was a problem hiding this comment.
Looks like there is a similar instance below.
There was a problem hiding this comment.
Ay, looks like gcc did not catch it because it was in false-conditional context.
| @@ -117,6 +117,7 @@ static int test_lib(void) | |||
| if (!TEST_true(shlib_load(path_crypto, &cryptolib)) | |||
There was a problem hiding this comment.
You may like to bump up the copyright year at the 2nd line of this file into '2017'
| @@ -117,6 +117,7 @@ static int test_lib(void) | |||
| if (!TEST_true(shlib_load(path_crypto, &cryptolib)) | |||
| || !TEST_true(shlib_load(path_ssl, &ssllib))) | |||
| goto end; | |||
| break; | |||
| case SSL_FIRST: | |||
| if (!TEST_true(shlib_load(path_ssl, &ssllib)) | |||
| || !TEST_true(shlib_load(path_crypto, &cryptolib))) | |||
There was a problem hiding this comment.
I am not sure if the || !TEST thing should be aligned to the ! above. But it seems doing so will help make the code at a low risk to exceed 80 boundary.
There was a problem hiding this comment.
(have we actually agreed on that, @mattcaswell?)
There was a problem hiding this comment.
Well let's say it's everywhere and used frequently and never been challenged.
There was a problem hiding this comment.
It's been question on the omc list a while ago, and I can't recall that we came to a conclusion, just that some of us like it and others don't. Ah well, we've let it go so far.
Now, to get that style into our source formatting script ;-)
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved. | |||
| * Copyright 2016-2017 The OpenSSL Project Authors. All Rights Reserved. | |||
There was a problem hiding this comment.
done.
I checked the Copyright headers in that directory and I wanted to ask if that one is correct:
x509_dup_cert_test.c: * Copyright (c) 2017 Oracle and/or its affiliates. All rights reserved.
There was a problem hiding this comment.
We would prefer just to have the OpenSSL author copyright, but it's not a requirement. The terms in the CLA let us do our redistribution terms. Ok?
There was a problem hiding this comment.
That copyright line from Oracle is confusing, and I think we should try to avoid it. It claims something in that file is copyright by Oracle and that we don't have a license for it.
In any case, you should not update copyright years that don't belong to you or the OpenSSL project.
There was a problem hiding this comment.
Surely, two copyright lines means dual copyright, no? As far as I know, that's valid.
There was a problem hiding this comment.
Where dual copyright means that there are 2 people having a copyright and both need to provide a license. Which is different from a dual license where 1 copyright holder makes it available under 2 licenses.
There was a problem hiding this comment.
Let's take this to the team mailing list.
|
commit 5511101 on master. |
Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from #3661)
This fixes a gcc warning about missing break (or fallthru comment).