Richard Levitte - VMS Whacker writes:

> Have you looked at lhash.h, and the DECLARE_LHASH*/IMPLEMENT_LHASH*
> macros?  That's how we have started to solve the type problems.  Yes,
> the generated functions contain some internal casting, but in such a
> way that they don't affect the code that use them, and thereby perform
> type safety in an acceptable manner.

I hadn't looked at lhash.h since the problems there only turned up when the
noise from the ASN.1 functions was out of the way.  In fact this looks
clean and maintainable.

> You haven't looked very much at the mainline, have you?  Those
> declarations do not exist any more in the 0.9.7 and 0.9.8-dev
> development lines.  As a matter of fact, the ASN.1 parser/generator

I had diffed asn1.h from 0.9.6g and 0.9.8 and found that most of the
missing prototypes were still there, thus concluding (obviously
prematurely) that the problem still exists for a large part.  Just having
rebuilt ntp-dev against the 0.9.7 branch indeed shows that while some
warnings are gone, a large number is still present.

> have been rewritten more or less entirely.  They use declaration and
> implementation macros much like the lhash ones I mentioned above.
> They are a little bit more complex, and found in crypto/asn1/asn1t.h.

I see that now, but the function pointers in ASN1_METHOD and many asn1.h
function declarations are still not prototyped.  The i2d, d2i, create and
destroy functions in ASN1_METHOD are similar, but not identical to
ASN1_i2d_func and friends from asn1t.h.  While ASN1_free_func and
ASN1_i2d_func could be used directly for destroy and i2d, the return types
for ASN1_new_func/create and ASN1_d2i_func/d2i differ: ASN1_VALUE */char *.

So one probably has to introduce compatibility typedefs in asn1.h, keeping
the char * return types, so callers don't have to be changed.

What approach would you prefer for this:

* typedef four new function types ASN1_compat_new_func etc. which match the
  old ASN1_METHOD members (and the function pointer arguments to several
  ASN.1 functions), although the compat_i2d and compat_free func are
  identical to their asn1t.h counterparts

* move the typedefs from asn1t.h to asn1.h and add only the two necessary
  compat typedefs

With this question out of the way, it will be easy to adapt my patch to use
them.

Unfortunately, unlike lhash.h, asn1.h/asn1t.h don't currently implement
generic i2d/d2i wrapper functions that would match
ASN1_i2d_func/ASN1_d2i_func and would avoid the need for function pointer
casting in calls to functions taking any i2d/d2i function.  Instead, only
type-specific functions are generated that still need to be cast to the
generic type.  While this can be changed, what about casts like the
following:

===================================================================
RCS file: apps/RCS/x509.c,v
retrieving revision 1.1
diff -up -r1.1 apps/x509.c
--- apps/x509.c 2002/08/08 21:13:58     1.1
+++ apps/x509.c 2002/08/16 18:33:21
@@ -931,7 +931,8 @@ bad:
                ah.meth=X509_asn1_meth();
 
                /* no macro for this one yet */
-               i=ASN1_i2d_bio(i2d_ASN1_HEADER,out,(unsigned char *)&ah);
+               i=ASN1_i2d_bio((ASN1_METHOD_I2D *)i2d_ASN1_HEADER,
+                              out,(unsigned char *)&ah);
                }
        else    {
                BIO_printf(bio_err,"bad output format specified for outfile\n");

Since ASN1_i2d_bio needs to take a pointer to a generic i2d function (and
thus its first arg changed from the unprototyped int (*i2d)() to either
ASN1_i2d_func *i2d or ASN1_compat_i2d_func *i2d), there are three possible
solutions:

* i2d_ASN1_HEADER needs to change to this generic type (i.e. taking an
  ASN1_VALUE * as first arg), an incompatibel change

* the cast is necessary

* a wrapper function matching the generic type is introduced (by what
  name?), but then the caller above would have to be changed

> Again, have you actually looked at the main line?  The EVP_MD_CTX
> structure has changed quite a bit, and instead of having a union of
> different algorithm-specific values, it now contains a void* called
> md_data.  If you then look at the EVP_MD functions, you'll see that
> they all take a EVP_MD_CTX*.  If you look at the code, say m_sha1.c,
> you'll see how it's handled, with type safety and NO casts.

Right: now there are only the sign and verify members of struct env_md_st
left to handle.  Since there are only three sets of those functions (RSA,
DSA, RSA_ASN1_OCTET_STRING), one might use an IMPLEMENT_EVP_PKEY_... macro
aproach as in lhash.h.  This is true for the init, update, and final
functions as well, which are currently implemented manually in m_*.c.
Unfortunately, the easy variant

#define IMPLEMENT_EVP_PKEY_method(name) \
static int sign (...) {
        // call name##_sign
};
int name##_verify (...) {
        // call name##_verify
};

doesn't work, since the RSA_sign_ASN1_OCTET_STRING etc. functions don't
match the name##_sign pattern.  I'll probably go for a (unfortunately
replicated) manual implementation in m_*.c.

> I trust it will be a bit smaller than your current patch :-).

Indeed: since some prototypes have already been introduced on mainline and
the need for many casts done.  I'll probably re-submit it in parts, one for
each separate set of function prototypes, i.e. asn1 functions, evp
sign/verify, and rsa callback functions.

One issue is not fixable at the moment: UI_ctrl takes a function pointer
argument which is currently unused; therefore it cannot be prototyped.  Any
idea what arguments this function will take?

        Rainer
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to