Re: Monolith compile verify.c

2020-06-25 Thread Michael Mueller
Thank you so much for the replies. I appreciate the effort, time, and
detail.

Turns out the warning is created by:
-Wextra

Only occurs in the older compiler (which we are required to use).

Removing the compiler flag worked.

Thanks again,
Mike

On Thu, Jun 25, 2020, 9:42 AM Michael Wojcik 
wrote:

> > From: openssl-users [mailto:openssl-users-boun...@openssl.org] On
> Behalf Of
> > Matt Caswell
> > Sent: Thursday, June 25, 2020 04:51
> >
> > On 24/06/2020 20:20, Michael Mueller wrote:
> > >
> > > Questions
> > >
> > > 1. Is the fix valid?
> >
> > Seems ok, but it looks like the compiler warning is a bit over zealous.
>
> It's valid, but I'd argue it's not a "fix".
>
> As Rich Salz pointed out, omitting initializers is permitted by the C
> standard. In fact it's endorsed, in the sense that the standard makes this
> behavior explicit (see e.g. ISO 9899-1999 6.7.8 #21 for arrays, or in the
> case of subobjects #19), and the Rationale (I'm referring to 5.10,
> published 2003) does not discourage its use, as it does for e.g. omitting
> braces for compound object initializers and relying on the mandated
> top-down-parse semantics.
>
> In his monumental /The New C Standard: An Economic and Cultural
> Commentary/ (2005), Derek M. Jones comments on this aspect of the language
> by noting some costs involved in requiring all members be explicitly
> initialized, and concludes: " Given these costs and the fact that
> developers are generally aware of the default behavior, there does not
> appear to be a worthwhile benefit in a guideline recommending that the
> behavior be made explicit." (1669) The GCC developers (like all C
> implementors) would have done well to read Jones.
>
> > We have the same pattern in many parts of the code I think without
> problems.
>
> Yes, as it should be.
>
> > > 2.  If fix is valid, what are the chances of getting the change
> accepted?
> >
> > Doubtful.
>
> And I'd say that's appropriate. Some of the GCC developers would prefer
> programmers use a language which is similar to, but distinctly not C,
> eschewing useful constructs and employing various others which are not part
> of the C language. OpenSSL is written in C.
>
> --
> Michael Wojcik
> Distinguished Engineer, Micro Focus
>
>


RE: Monolith compile verify.c

2020-06-25 Thread Michael Wojcik
> From: openssl-users [mailto:openssl-users-boun...@openssl.org] On Behalf Of
> Matt Caswell
> Sent: Thursday, June 25, 2020 04:51
>
> On 24/06/2020 20:20, Michael Mueller wrote:
> >
> > Questions
> >
> > 1. Is the fix valid?
>
> Seems ok, but it looks like the compiler warning is a bit over zealous.

It's valid, but I'd argue it's not a "fix".

As Rich Salz pointed out, omitting initializers is permitted by the C standard. 
In fact it's endorsed, in the sense that the standard makes this behavior 
explicit (see e.g. ISO 9899-1999 6.7.8 #21 for arrays, or in the case of 
subobjects #19), and the Rationale (I'm referring to 5.10, published 2003) does 
not discourage its use, as it does for e.g. omitting braces for compound object 
initializers and relying on the mandated top-down-parse semantics.

In his monumental /The New C Standard: An Economic and Cultural Commentary/ 
(2005), Derek M. Jones comments on this aspect of the language by noting some 
costs involved in requiring all members be explicitly initialized, and 
concludes: " Given these costs and the fact that developers are generally aware 
of the default behavior, there does not appear to be a worthwhile benefit in a 
guideline recommending that the behavior be made explicit." (1669) The GCC 
developers (like all C implementors) would have done well to read Jones.

> We have the same pattern in many parts of the code I think without problems.

Yes, as it should be.

> > 2.  If fix is valid, what are the chances of getting the change accepted?
>
> Doubtful.

And I'd say that's appropriate. Some of the GCC developers would prefer 
programmers use a language which is similar to, but distinctly not C, eschewing 
useful constructs and employing various others which are not part of the C 
language. OpenSSL is written in C.

--
Michael Wojcik
Distinguished Engineer, Micro Focus



Re: Monolith compile verify.c

2020-06-25 Thread Matt Caswell



On 24/06/2020 20:20, Michael Mueller wrote:
> 
> Questions
> 
> 1. Is the fix valid?

Seems ok, but it looks like the compiler warning is a bit over zealous.
We have the same pattern in many parts of the code I think without problems.

> 
> 2.  If fix is valid, what are the chances of getting the change accepted?

Doubtful.

Matt



Re: Monolith compile verify.c

2020-06-24 Thread Salz, Rich via openssl-users
C mandates that any “missing” initializers are as if 0/null were present.


{NULL, -1, 'Q', "unused end of list"} this is the change I’d like to offer



Turn off the warning.


Monolith compile verify.c

2020-06-24 Thread Michael Mueller
I'm compiling verify.c as a monolith from 1.1.1g.

Using gcc 4.8.5 I have no problems.

Using gcc 4.3.4 I get a warning which we treat as an error:

gcc -c -Wall -Werror -Wextra -idirafter ../../inc -idirafter
../../../../OPENSSL/include/SUSE-Linux -DMONOLITH -DNO_ASN1_OLD -DLINUX -o
../../obj/verify.o ../../src/verify.c

cc1: warnings being treated as errors

../../src/verify.c:64: error: missing initializer

../../src/verify.c:64: error: (near initialization for
‘verify_options[45].retval’)



I found the source of my problem and created a fix that I could toggle when
testing with different compilers. The fix compiles cleanly with both gcc
compilers mentioned above.

I have not tried it with Visual Studio yet.


const OPTIONS verify_options[] = {

{OPT_HELP_STR, 1, '-', "Usage: %s [options] cert.pem...\n"},

{OPT_HELP_STR, 1, '-', "Valid options are:\n"},

{"help", OPT_HELP, '-', "Display this summary"},

{"verbose", OPT_VERBOSE, '-',

"Print extra information about the operations being performed."},

{"CApath", OPT_CAPATH, '/', "A directory of trusted certificates"},

{"CAfile", OPT_CAFILE, '<', "A file of trusted certificates"},

{"no-CAfile", OPT_NOCAFILE, '-',

 "Do not load the default certificates file"},

{"no-CApath", OPT_NOCAPATH, '-',

 "Do not load certificates from the default certificates directory"},

{"untrusted", OPT_UNTRUSTED, '<', "A file of untrusted certificates"},

{"trusted", OPT_TRUSTED, '<', "A file of trusted certificates"},

{"CRLfile", OPT_CRLFILE, '<',

"File containing one or more CRL's (in PEM format) to load"},

{"crl_download", OPT_CRL_DOWNLOAD, '-',

"Attempt to download CRL information for this certificate"},

{"show_chain", OPT_SHOW_CHAIN, '-',

"Display information about the certificate chain"},

{"nameopt", OPT_NAMEOPT, 's', "Various certificate name options"},

OPT_V_OPTIONS,

#ifndef OPENSSL_NO_ENGINE

{"engine", OPT_ENGINE, 's', "Use engine, possibly a hardware device"},

#endif

#if 0

{NULL, -1, 'Q', "unused end of list"} this is the change I’d like to
offer

#else

{NULL} this is the current code

#endif

};


Questions

1. Is the fix valid?

2.  If fix is valid, what are the chances of getting the change accepted?

Thanks

Mike