On 06/18/2015 12:45 AM, Malahal Naineni wrote:
> I didn't see GLUSTERFSAL_UP_Thread() returning anything other than
> NULL/0. I think passing NULL to pthread_join is even cleaner!
>

Agree that current GLUSTERFSAL_UP_THREAD returns only NULL. But 
considering the fact that we shall add more upcall events and may be an 
option to decide if upcall is required or not which may lead to multiple 
exit cases, I had thought it may be good to capture the return status of 
thread exit, if at all passed.

> Regards, Malahal.
>
> Malahal Naineni [mala...@us.ibm.com] wrote:
>> Soumya Koduri [skod...@redhat.com] wrote:
>>> Hi Kaleb/Malahal,
>>>
>>> Request you to merge below FSAL_GLUSTER patches into V2.2-stable branch -
>>>
>>> 366f71c - FSAL_GLUSTER: Fixed an issue with dereferencing a NULL ponter
>>
>> I just looked at the patch from your other mail. I have few questions on
>> this patch.
>>
>> 1. I am not sure why you declared retval as int *. Does the up_thread
>>     exists with an "int *"? Most threads just exit with an integer, so
>>     declaring just "int retval" or a later casting would have been
>>     suffice.
>>
>> 2. Technically, passed in OUT argument is updated by pthead_join() only
>>     on success, so it should be valid only if pthead_join() returned
>>     success (usually this should NOT fail though).
>>
Right. But even on success, "retval" need not be set until the thread 
exits with a value other than NULL.

 From the manpage,
 >>>
        If retval is not NULL, then pthread_join() copies the exit 
status of the target thread (i.e., the value that the target thread 
supplied to pthread_exit(3)) into the location pointed to by *retval. 
If the target thread was canceled, then PTHREAD_CANCELED is placed in 
*retval.
<<<<<

So to check if the retval is not NULL or if thread in fact had exit with 
a value, had declared it as pointer.


>> All in all, the code should be something like this:
>>
>> int retval;
>>
>> err = pthread_join(up_thread, (void**)&retval);
>> if (err)
>>      LogCrit(XX, "pthread_join faild: %d", err);
>> else
>>      LogDebug(XX, "upthread exited with: %d", retval);
>>
>> Checking for '*retval' is only useful if your up_thread exited with
>> something like pthread_exit(&some_var), but I don't think it does that.
>>
>> Regards, Malahal.
>> PS: I am just looking at the manpage, no direct experince!  Did you ever
>> see the system printing a sane "Up_thread join returned value %d"
>> message?
No at present, we do not see these messages as UP_THREAD returns NULL on 
exit.

Thanks,
Soumya
>>
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> Nfs-ganesha-devel mailing list
>> Nfs-ganesha-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
>>
>

------------------------------------------------------------------------------
_______________________________________________
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Reply via email to