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