apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lucian Adrian Grijincu" <lucian.griji...@gmail.com>
Subject Re: some simplifications for the Netware apr_thread_create code
Date Sun, 14 Oct 2007 17:14:00 GMT
On 10/14/07, William A. Rowe, Jr. <wrowe@rowe-clan.net> 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/%3C20011016232705.3884.qmail@icarus.apache.org%3E
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

Mime
View raw message