Re: [openssl.org #781] [PATCH] NetWare Support for OpenSSL 0.9.7
I did not realize that ERR_remove_state() worked that way. In looking at the code, it appears that it does indeed delete the table making ERR_free_state_table() unnecessary in properly coded applications. Thanks. Verdon>>> "Richard Levitte via RT" <[EMAIL PROTECTED]> 6/28/2004 5:47:58 AM >>> Just a comment on what I just wrote...[levitte - Mon Jun 28 13:37:50 2004]:> I've looked back at the ERR_free_state_table() discussion, and I can't > find any message where we came to a resolution. Looking at the patch > right now, I still fail to see what that function does that > ERR_remove_state() doesn't. It should be noted that ERR_remove_state() > *does* remove the whole table when there are no more references to it. > The way it's supposed to work is that it should be called at the end of > each thread (including the main one). My stand on this issue hasn't > changed since we discussed it last.If ERR_remove_state() doesn't work as it should (as said above), then it's a bug in ERR_remove_state() and should be treated that way, and therefore result in a fix of ERR_remove_state(). IMHO.-- Richard Levitte[EMAIL PROTECTED]
Re: [openssl.org #781] [PATCH] NetWare Support for OpenSSL 0.9.7
__ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #781] [PATCH] NetWare Support for OpenSSL 0.9.7
Richard, Any chance yet to reconsider applying this patch to 0.9.7? Verdon >>> [EMAIL PROTECTED] 12/2/2003 3:38:32 PM >>> Addressing the points in order: ERR_free_state_table() is not meant to do the same as ERR_remove_state(). It is indeed meant to clean up the entire table and is used for process cleanup, not thread cleanup. NetWare will clean up process resources, but does complain when you don't clean up after yourself. The whole issue of library cleanup is best addressed, however, as a separate topic rather than followup on this particular patch so I will do so in a separate email. I was not aware that crypto/des/read_pwd.c was not in use anymore. This change was pulled forward from our 0.9.6 changes. Thanks for the info. The removal of EVP_MD_CTX_cleanup() in crypto/mdc2/mdc2test.c was indeed an oversight. Yes, it is our intent to continue to support these changes in future versions of OpenSSL including building and testing them in each release (and pre-release) of OpenSSL. Thank you so VERY much for your consideration of this work! Verdon Walker (801) 861-2633 [EMAIL PROTECTED] Novell, Inc., the leading provider of information solutions http://www.novell.com >>> [EMAIL PROTECTED] 11/28/2003 5:39:44 AM >>> [EMAIL PROTECTED] - Thu Nov 27 09:23:09 2003]: > This patch adds support for the NetWare OS to OpenSSL 0.9.7. After > applying this patch, the instructions for building for NetWare can be > found in the "INSTALL.NW" file in the root directory. This patch > attempts to minimize impact on other platforms through the judicious > use > of the preprocessor guard "OPENSSL_SYS_NETWARE". It does introduce one > new general purpose function "ERR_free_state_table" which we found > useful when cleaning up threads. I'm not sure I understand the need for ERR_free_state_table(). It seems to me that ERR_remove_state() performs the same operation, but makes sure that only the state of the current thread is cleaned (int_thread_del_item() will free that whole hash table when the last thread runs it). To be perfectly clear, it seems to me like ERR_free_state_table() would be disastrous to use in a threaded environment, or at least very potentially so. I will therefore reject the changes to crypto/err/err. c, crypto/err/err.h and util/libeay.num. There are a few more issues, although comparatively minor: 1. Do you know that crypto/des/read_pwd.c isn't used at all any more? It can safely be ignored. I will therefore reject your changes to that file. 2. In crypto/mdc2/mdc2test.c, I notice you removed the call to EVP_MD_CTX_cleanup() on line 142. Was there a reason for that, or is it just a typo? 3. Will you help us maintain these changes in future versions of OpenSSL, or is there a risk that the new files will age quickly? > I have applied the patch to the latest 0.9.7c source and built for > both > NetWare and Windows. The test suite runs correctly on both platforms > with the patch applied. > > I understand that active development on the 0.9.7 code is limited. > These changes, however, have minimal impact on existing code and I > hope > you will consider them. If the patch is not accepted for 0.9.7, I > request that this patch file be added to the "Contributions" page in > the > same manner as the 0.9.6 NetWare patch. I will place the patch kit, with the rejected changes removed, in the contribution directory for now. I'm keeping this ticket open and will await your answer to my issues. I will also check with the other developpers if they have some issues with this change. After that, I will reconsider the application of the change. I'm making no promisses, though. -- Richard Levitte [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #781] [PATCH] NetWare Support for OpenSSL 0.9.7
Addressing the points in order: ERR_free_state_table() is not meant to do the same as ERR_remove_state(). It is indeed meant to clean up the entire table and is used for process cleanup, not thread cleanup. NetWare will clean up process resources, but does complain when you don't clean up after yourself. The whole issue of library cleanup is best addressed, however, as a separate topic rather than followup on this particular patch so I will do so in a separate email. I was not aware that crypto/des/read_pwd.c was not in use anymore. This change was pulled forward from our 0.9.6 changes. Thanks for the info. The removal of EVP_MD_CTX_cleanup() in crypto/mdc2/mdc2test.c was indeed an oversight. Yes, it is our intent to continue to support these changes in future versions of OpenSSL including building and testing them in each release (and pre-release) of OpenSSL. Thank you so VERY much for your consideration of this work! Verdon Walker (801) 861-2633 [EMAIL PROTECTED] Novell, Inc., the leading provider of information solutions http://www.novell.com >>> [EMAIL PROTECTED] 11/28/2003 5:39:44 AM >>> [EMAIL PROTECTED] - Thu Nov 27 09:23:09 2003]: > This patch adds support for the NetWare OS to OpenSSL 0.9.7. After > applying this patch, the instructions for building for NetWare can be > found in the "INSTALL.NW" file in the root directory. This patch > attempts to minimize impact on other platforms through the judicious > use > of the preprocessor guard "OPENSSL_SYS_NETWARE". It does introduce one > new general purpose function "ERR_free_state_table" which we found > useful when cleaning up threads. I'm not sure I understand the need for ERR_free_state_table(). It seems to me that ERR_remove_state() performs the same operation, but makes sure that only the state of the current thread is cleaned (int_thread_del_item() will free that whole hash table when the last thread runs it). To be perfectly clear, it seems to me like ERR_free_state_table() would be disastrous to use in a threaded environment, or at least very potentially so. I will therefore reject the changes to crypto/err/err. c, crypto/err/err.h and util/libeay.num. There are a few more issues, although comparatively minor: 1. Do you know that crypto/des/read_pwd.c isn't used at all any more? It can safely be ignored. I will therefore reject your changes to that file. 2. In crypto/mdc2/mdc2test.c, I notice you removed the call to EVP_MD_CTX_cleanup() on line 142. Was there a reason for that, or is it just a typo? 3. Will you help us maintain these changes in future versions of OpenSSL, or is there a risk that the new files will age quickly? > I have applied the patch to the latest 0.9.7c source and built for > both > NetWare and Windows. The test suite runs correctly on both platforms > with the patch applied. > > I understand that active development on the 0.9.7 code is limited. > These changes, however, have minimal impact on existing code and I > hope > you will consider them. If the patch is not accepted for 0.9.7, I > request that this patch file be added to the "Contributions" page in > the > same manner as the 0.9.6 NetWare patch. I will place the patch kit, with the rejected changes removed, in the contribution directory for now. I'm keeping this ticket open and will await your answer to my issues. I will also check with the other developpers if they have some issues with this change. After that, I will reconsider the application of the change. I'm making no promisses, though. -- Richard Levitte [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
