On 07/23/2006 07:54 PM, Ben Laurie wrote: > Ruediger Pluem wrote: > >>On 07/23/2006 02:10 PM, Ben Laurie wrote: >> >>>Joe Orton wrote: >> >>>>- use APR apr_file_* not ANSI C fopen, >>> >>>I need a FILE *. >> >>Maybe you could use BIO_new_file / PEM_read_bio_PKCS7 as it is done in similar >>situations in other places of mod_ssl. > > > Why?
Because it would be consistent with the other part of the code. Why should we use different approaches for similar problems? > > >>>>- the server doesn't even start up (without the new stuff configured): >>>> >>>>[Sun Jul 23 10:25:14 2006] [info] Loading certificate & private key of >>>>SSL-aware server >>>>[Sun Jul 23 10:25:14 2006] [error] Can't open \x80\x94| >>> >>>Hmmm. Can't reproduce this (on the trunk). >> >>This is pure luck. You should init pkcs7 to NULL in modssl_ctx_init as done >>for the other >>elements of this struct. > > > Sure. You are allowed to just commit the fix, you know? Sure I know, but during the short time I have been around here it was usually the way that the committer of a patch either discussed / rejected review comments on its commit or fixed them by itself. That is what I understand on CTR. Anyway fixed in r424821, plus some style fixes in r424823. > > >>>>- fix the compiler warning (and perhaps hence the above): >>>> >>>>ssl_util.c: In function 'ssl_read_pkcs7': >>>>ssl_util.c:271: warning: 'certs' may be used uninitialized in this function >>> >>>This won't fix the above. certs is actually only uninitialized if death >>>is about to occur. >> >>Then please do >> >> STACK_OF(X509) *certs = NULL; >> >>instead of >> >> STACK_OF(X509) *certs; > > > I am aware of how to fix it (and, indeed, have done so), I'm just > pointing out that it won't fix the problem. I does fix the problem because NULL is a well defined value and *certs is now initialized. > > BTW, it would be nice to be able to run -Wall -Werror, as I normally do, > but it seems there's code in the repo that doesn't survive that test. > That should not be the case. Maybe Joe can help here, because I guess he always compiles at least with -Wall if not with -Werror. Regards Rüdiger
