Re: [openssl.org #85] 0.9.7 prototype constification problems
On Mon, Jun 17, 2002 at 07:02:45PM +0100, Ben Laurie wrote: Avery Pennarun via RT wrote: On Mon, Jun 17, 2002 at 11:19:31AM +0200, Bodo Moeller wrote: Good question, but this problem does not appear to apply to C, and anyway it only makes *existing* code uglier -- for new code, the modified API makes more sense (the encoded data can be in 'const unsigned char' arrays all the time). If you don't want #ifdefs and casts throughout your code, please consider hiding this in wrapper functions. Hmm, okay, I'm not really convinced, but now that I see it doesn't cause C programs to not compile, I think it's okay to accept the small number of openssl-using C++ programs that will need changes because of this. Hmm, well, I'm not, because all right-thinking projects have a zero-warnings policy. OpenSSL included. The old API is not perfect when the data is located in a 'const' array, the new API is not perfect when the data is located in a non-'const' array. C++ code can be written to match either the old or the new API; with the wrong API, you can have either zero warnings or zero casts, not both. As those functions are not really supposed to modify the data, the 'const' variant appears to be the right one. -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
Bodo Moeller wrote: On Mon, Jun 17, 2002 at 07:02:45PM +0100, Ben Laurie wrote: Avery Pennarun via RT wrote: On Mon, Jun 17, 2002 at 11:19:31AM +0200, Bodo Moeller wrote: Good question, but this problem does not appear to apply to C, and anyway it only makes *existing* code uglier -- for new code, the modified API makes more sense (the encoded data can be in 'const unsigned char' arrays all the time). If you don't want #ifdefs and casts throughout your code, please consider hiding this in wrapper functions. Hmm, okay, I'm not really convinced, but now that I see it doesn't cause C programs to not compile, I think it's okay to accept the small number of openssl-using C++ programs that will need changes because of this. Hmm, well, I'm not, because all right-thinking projects have a zero-warnings policy. OpenSSL included. The old API is not perfect when the data is located in a 'const' array, the new API is not perfect when the data is located in a non-'const' array. C++ code can be written to match either the old or the new API; with the wrong API, you can have either zero warnings or zero casts, not both. As those functions are not really supposed to modify the data, the 'const' variant appears to be the right one. Fair enough. Cheers, Ben. -- http://www.apache-ssl.org/ben.html http://www.thebunker.net/ There is no limit to what a man can do or how far he can go if he doesn't mind who gets the credit. - Robert Woodruff __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
Avery Pennarun [EMAIL PROTECTED]: On Thu, Jun 13, 2002 at 01:26:42PM +0200, Bodo Moeller via RT wrote: [[EMAIL PROTECTED] - Thu Jun 6 18:39:34 2002]: It appears the openssl guys goofed in 0.97beta. The prototype for the d2i_RSAPrivateKey function in 0.9.6c, which I use, is like this: d2i_RSAPrivateKey(RSA **a, unsigned char **pp, long length); ie., without a const on the second parameter. The const should have been done like this (I think): const unsigned char * const *pp ...which would be entirely backwards-compatible with old versions of openssl. Could you explain why you think this would improve compatibility (compared with 'const unsigned char **pp', which is how 'pp' should be declared in 0.9.7-beta1, although the exact definition is hidden behind macros)? Any variable that could be passed to a function taking unsigned char ** could also be passed to a function taking const unsigned char * const *. Thus, using the above declaration would make openssl's api 100% source-compatible with previous versions, while promising an increased level of constness. Using const unsigned char **, however, is not 100% api-compatible, because you can't safely pass an unsigned char ** to it, for complicated reasons explained in the URL I sent earlier. [http://www.geocrawler.com/archives/3/362/1997/9/0/2036587/] I see, so this is an C++ speciality. As far as C is concerned, the types should be incompatible between any of these three variants, but compilers tend not to look that close (and otherwise you'd use a cast). Writing code that tries to change a 'const char' object as shown at the above URL is possible -- the actual assignment is illegal, but in general the compiler cannot catch this. In fact the second 'const' would be wrong because the pointer that 'pp' points to is updated to reflect how much of the encoded data has been processed by the d2i_... function. I.e., it is not at all constant. Now, you caught me there. The change I proposed earlier (adding an additional const) would fix API compatibility, but promises a level of constness that you're not actually providing. Unfortunately, I don't know a way around it: the problem is that the beta1 level of constness looks right, but doesn't _actually_ promise the constness that it looks like it does, due to the crazy hackery described in the URL I sent earlier. Prototypes alone don't really guarantee constness anyway. If a function is passed a 'const int *' pointer and knows somehow that the object pointed to is not 'const' (i.e. the pointer was 'int *' originally), it may cast the pointer back to 'int *' and modify the object. It also makes the API incompatible with older versions. I can't pass the address of unsigned char *p to the new function. I guess you'll have to include a cast to the new type depending on the value of OPENSSL_VERSION_NUMBER. I know this is ugly, but I don't think we should un-constify the prototype to avoid such problems. (In fact we'll have to make various other prototype changes to clean up the API, for example 'int' quite often should be 'size_t' etc.) -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
On Sat, Jun 15, 2002 at 12:14:00PM +0200, Bodo Moeller wrote: Using const unsigned char **, however, is not 100% api-compatible, because you can't safely pass an unsigned char ** to it, for complicated reasons explained in the URL I sent earlier. [http://www.geocrawler.com/archives/3/362/1997/9/0/2036587/] I see, so this is an C++ speciality. As far as C is concerned, the types should be incompatible between any of these three variants, but compilers tend not to look that close (and otherwise you'd use a cast). Writing code that tries to change a 'const char' object as shown at the above URL is possible -- the actual assignment is illegal, but in general the compiler cannot catch this. I don't understand why you say this is C++-specific: I think C does const in the same way. My original suggestion (two consts) is not incompatible with either of the others, it just promises a level of constness that you're not actually going to provide. Unfortunately, I don't know a way around it: the problem is that the beta1 level of constness looks right, but doesn't _actually_ promise the constness that it looks like it does, due to the crazy hackery described in the URL I sent earlier. Prototypes alone don't really guarantee constness anyway. If a function is passed a 'const int *' pointer and knows somehow that the object pointed to is not 'const' (i.e. the pointer was 'int *' originally), it may cast the pointer back to 'int *' and modify the object. Prototypes are really just a clue for the user and implementer of a class: a program would run exactly the same if const were globally #defined to be meaningless. If you're not concerned about what you promise the user (and a good comment above the prototype can repair that promise anyway) then use my original suggestion const unsigned char * const * which will happily accept parameters of any of these types: unsigned char ** const unsigned char ** const unsigned char * const * ...and then typecast it _yourself_ inside the function. This maintains API compatibility and lets you pass const unsigned char ** objects, which is what you presumably wanted. It also makes the API incompatible with older versions. I can't pass the address of unsigned char *p to the new function. I guess you'll have to include a cast to the new type depending on the value of OPENSSL_VERSION_NUMBER. I know this is ugly, but I don't think we should un-constify the prototype to avoid such problems. Oh, argh, no! Please, please don't do this. So far we've managed to avoid all such things and wvstreams goes back to quite old revs of openssl. What's the point of an api cleanup that makes user code uglier? (In fact we'll have to make various other prototype changes to clean up the API, for example 'int' quite often should be 'size_t' etc.) This might produce warnings, but never errors, I think. Have fun, Avery __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
Avery Pennarun [EMAIL PROTECTED]: On Thu, Jun 13, 2002 at 01:26:42PM +0200, Bodo Moeller via RT wrote: [[EMAIL PROTECTED] - Thu Jun 6 18:39:34 2002]: It appears the openssl guys goofed in 0.97beta. The prototype for the d2i_RSAPrivateKey function in 0.9.6c, which I use, is like this: d2i_RSAPrivateKey(RSA **a, unsigned char **pp, long length); ie., without a const on the second parameter. The const should have been done like this (I think): const unsigned char * const *pp ...which would be entirely backwards-compatible with old versions of openssl. Could you explain why you think this would improve compatibility (compared with 'const unsigned char **pp', which is how 'pp' should be declared in 0.9.7-beta1, although the exact definition is hidden behind macros)? Any variable that could be passed to a function taking unsigned char ** could also be passed to a function taking const unsigned char * const *. Thus, using the above declaration would make openssl's api 100% source-compatible with previous versions, while promising an increased level of constness. Using const unsigned char **, however, is not 100% api-compatible, because you can't safely pass an unsigned char ** to it, for complicated reasons explained in the URL I sent earlier. [http://www.geocrawler.com/archives/3/362/1997/9/0/2036587/] I see, so this is an C++ speciality. As far as C is concerned, the types should be incompatible between any of these three variants, but compilers tend not to look that close (and otherwise you'd use a cast). Writing code that tries to change a 'const char' object as shown at the above URL is possible -- the actual assignment is illegal, but in general the compiler cannot catch this. In fact the second 'const' would be wrong because the pointer that 'pp' points to is updated to reflect how much of the encoded data has been processed by the d2i_... function. I.e., it is not at all constant. Now, you caught me there. The change I proposed earlier (adding an additional const) would fix API compatibility, but promises a level of constness that you're not actually providing. Unfortunately, I don't know a way around it: the problem is that the beta1 level of constness looks right, but doesn't _actually_ promise the constness that it looks like it does, due to the crazy hackery described in the URL I sent earlier. Prototypes alone don't really guarantee constness anyway. If a function is passed a 'const int *' pointer and knows somehow that the object pointed to is not 'const' (i.e. the pointer was 'int *' originally), it may cast the pointer back to 'int *' and modify the object. It also makes the API incompatible with older versions. I can't pass the address of unsigned char *p to the new function. I guess you'll have to include a cast to the new type depending on the value of OPENSSL_VERSION_NUMBER. I know this is ugly, but I don't think we should un-constify the prototype to avoid such problems. (In fact we'll have to make various other prototype changes to clean up the API, for example 'int' quite often should be 'size_t' etc.) -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
On Sat, Jun 15, 2002 at 12:14:00PM +0200, Bodo Moeller wrote: Using const unsigned char **, however, is not 100% api-compatible, because you can't safely pass an unsigned char ** to it, for complicated reasons explained in the URL I sent earlier. [http://www.geocrawler.com/archives/3/362/1997/9/0/2036587/] I see, so this is an C++ speciality. As far as C is concerned, the types should be incompatible between any of these three variants, but compilers tend not to look that close (and otherwise you'd use a cast). Writing code that tries to change a 'const char' object as shown at the above URL is possible -- the actual assignment is illegal, but in general the compiler cannot catch this. I don't understand why you say this is C++-specific: I think C does const in the same way. My original suggestion (two consts) is not incompatible with either of the others, it just promises a level of constness that you're not actually going to provide. Unfortunately, I don't know a way around it: the problem is that the beta1 level of constness looks right, but doesn't _actually_ promise the constness that it looks like it does, due to the crazy hackery described in the URL I sent earlier. Prototypes alone don't really guarantee constness anyway. If a function is passed a 'const int *' pointer and knows somehow that the object pointed to is not 'const' (i.e. the pointer was 'int *' originally), it may cast the pointer back to 'int *' and modify the object. Prototypes are really just a clue for the user and implementer of a class: a program would run exactly the same if const were globally #defined to be meaningless. If you're not concerned about what you promise the user (and a good comment above the prototype can repair that promise anyway) then use my original suggestion const unsigned char * const * which will happily accept parameters of any of these types: unsigned char ** const unsigned char ** const unsigned char * const * ...and then typecast it _yourself_ inside the function. This maintains API compatibility and lets you pass const unsigned char ** objects, which is what you presumably wanted. It also makes the API incompatible with older versions. I can't pass the address of unsigned char *p to the new function. I guess you'll have to include a cast to the new type depending on the value of OPENSSL_VERSION_NUMBER. I know this is ugly, but I don't think we should un-constify the prototype to avoid such problems. Oh, argh, no! Please, please don't do this. So far we've managed to avoid all such things and wvstreams goes back to quite old revs of openssl. What's the point of an api cleanup that makes user code uglier? (In fact we'll have to make various other prototype changes to clean up the API, for example 'int' quite often should be 'size_t' etc.) This might produce warnings, but never errors, I think. Have fun, Avery __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
On Thu, Jun 13, 2002 at 01:26:42PM +0200, Bodo Moeller via RT wrote: [[EMAIL PROTECTED] - Thu Jun 6 18:39:34 2002]: It appears the openssl guys goofed in 0.97beta. The prototype for the d2i_RSAPrivateKey function in 0.9.6c, which I use, is like this: d2i_RSAPrivateKey(RSA **a, unsigned char **pp, long length); ie., without a const on the second parameter. The const should have been done like this (I think): const unsigned char * const *pp ...which would be entirely backwards-compatible with old versions of openssl. Could you explain why you think this would improve compatibility (compared with 'const unsigned char **pp', which is how 'pp' should be declared in 0.9.7-beta1, although the exact definition is hidden behind macros)? Any variable that could be passed to a function taking unsigned char ** could also be passed to a function taking const unsigned char * const *. Thus, using the above declaration would make openssl's api 100% source-compatible with previous versions, while promising an increased level of constness. Using const unsigned char **, however, is not 100% api-compatible, because you can't safely pass an unsigned char ** to it, for complicated reasons explained in the URL I sent earlier. In fact the second 'const' would be wrong because the pointer that 'pp' points to is updated to reflect how much of the encoded data has been processed by the d2i_... function. I.e., it is not at all constant. Now, you caught me there. The change I proposed earlier (adding an additional const) would fix API compatibility, but promises a level of constness that you're not actually providing. Unfortunately, I don't know a way around it: the problem is that the beta1 level of constness looks right, but doesn't _actually_ promise the constness that it looks like it does, due to the crazy hackery described in the URL I sent earlier. It also makes the API incompatible with older versions. I can't pass the address of unsigned char *p to the new function. On the other hand, I can't pass the address of const unsigned char *p to the old-style function. Neither situation is desirable... but it looks like you might have to just use two different functions with two different APIs, unfortunately. I don't know another way out. Have fun, Avery __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #85] 0.9.7 prototype constification problems
On Thu, Jun 13, 2002 at 01:26:42PM +0200, Bodo Moeller via RT wrote: [[EMAIL PROTECTED] - Thu Jun 6 18:39:34 2002]: It appears the openssl guys goofed in 0.97beta. The prototype for the d2i_RSAPrivateKey function in 0.9.6c, which I use, is like this: d2i_RSAPrivateKey(RSA **a, unsigned char **pp, long length); ie., without a const on the second parameter. The const should have been done like this (I think): const unsigned char * const *pp ...which would be entirely backwards-compatible with old versions of openssl. Could you explain why you think this would improve compatibility (compared with 'const unsigned char **pp', which is how 'pp' should be declared in 0.9.7-beta1, although the exact definition is hidden behind macros)? Any variable that could be passed to a function taking unsigned char ** could also be passed to a function taking const unsigned char * const *. Thus, using the above declaration would make openssl's api 100% source-compatible with previous versions, while promising an increased level of constness. Using const unsigned char **, however, is not 100% api-compatible, because you can't safely pass an unsigned char ** to it, for complicated reasons explained in the URL I sent earlier. In fact the second 'const' would be wrong because the pointer that 'pp' points to is updated to reflect how much of the encoded data has been processed by the d2i_... function. I.e., it is not at all constant. Now, you caught me there. The change I proposed earlier (adding an additional const) would fix API compatibility, but promises a level of constness that you're not actually providing. Unfortunately, I don't know a way around it: the problem is that the beta1 level of constness looks right, but doesn't _actually_ promise the constness that it looks like it does, due to the crazy hackery described in the URL I sent earlier. It also makes the API incompatible with older versions. I can't pass the address of unsigned char *p to the new function. On the other hand, I can't pass the address of const unsigned char *p to the old-style function. Neither situation is desirable... but it looks like you might have to just use two different functions with two different APIs, unfortunately. I don't know another way out. Have fun, Avery __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]