On 10/14/07, William A. Rowe, Jr. <[EMAIL PROTECTED]> wrote: > Lucian Adrian Grijincu wrote: > > isn't this why they invented comments? > > > > why [...] when you can do: > > return x; //and place a witty comment here? > > > > saves a cmp and a jump. > > Just forewarn - // isn't portable, please don't litter :) More than happy to > see > /* */ style comments. >
Sorry for being ignorant :). I'll try to remember that. > In your patch the adjustments are good at first glance, if none of the > netware folks > look at them I'll catch up soon. You might want to create an apr 'small code > cleanups' > tracking bug, and drop these in as (individual!) attachments for easy review. > http://issues.apache.org/bugzilla/show_bug.cgi?id=43620 But after looking at the code, I'm not whether I should attach the patch or not. Here is why: the code looks like this: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/netware/thread.c?view=markup (*new)->ctx = NXContextAlloc( some_uninteresting_list_of_parameters ); stat = NXContextSetName( some_uninteresting_list_of_parameters ); stat = NXThreadCreate( some_uninteresting_list_of_parameters ); if(stat==0) return APR_SUCCESS; return(stat);// if error Disclaimer: I have no idea what Netware is, does, eats, etc. I haven't coded to it's API. But through a simple Google Search I've found that when NXThreadCreate fails other projects free the context allocated at the beginning of this little snip of code. http://mail-archives.apache.org/mod_mbox/httpd-cvs/200110.mbox/[EMAIL PROTECTED] search in the page when it mentions NXThreadCreate. This looks as a better approach. (*new)->ctx = NXContextAlloc( some_uninteresting_list_of_parameters ); stat = NXContextSetName( some_uninteresting_list_of_parameters ); stat = NXThreadCreate( some_uninteresting_list_of_parameters ); if(stat==0) return APR_SUCCESS; NXContextFree((*new)->ctx); /*Free your resources*/ return(stat); The internal pool could be freed before the return as well (even though it will get freed later by the parent pool). Also this context should be freed in other functions as well (e.g. apr_thread_exit). I think someone with Netware experience should review this code. -- Lucian
