Re: [Nfs-ganesha-devel] Request to merge patches to V2.2-stable branch
Soumya Koduri [skod...@redhat.com] wrote: I thought pthread_exit() always returns a pointer which gets assigned to retval of pthread_join(). I assume this is the flow -- pthread_exit() does take a void *, since it is void *, it is up to you whether to really pass some pointer or some casted integer. In fact, PTHREAD_CANCELED is actually ((void *) -1) pthread_join (thread, void **retval_join) pthread_exit (void *retval_exit) On exit, *retval_join = retval_exit; I tried a sample program as per your suggestion. int ret1; pthread_join (thread, (void **)ret1); int ret2 = 1; pthread_exit (ret2); After pthread_join, ret1 has address of ret2 instead of its value. i.e, this led to ret1 = ret2; Please correct me if I am missing something. Had you used pthread_exit((void*)ret2) or better pthread_exit((void*)1), ret1 would have the integer 1. This seems to be the standard practice. If you really want to pass a pointer to pthread_exit(), make sure that the thread that called pthread_join() will be able to access such memory. Too much hassle, that is why most people just cast integers if they just need integer returns. It is up to you but I feel that the code gets unnecessarily complicated by returning a real pointer from the upcall thread. Regards, Malahal. -- ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
Re: [Nfs-ganesha-devel] Request to merge patches to V2.2-stable branch
On 06/18/2015 10:33 PM, Malahal Naineni wrote: Soumya Koduri [skod...@redhat.com] wrote: I thought pthread_exit() always returns a pointer which gets assigned to retval of pthread_join(). I assume this is the flow -- pthread_exit() does take a void *, since it is void *, it is up to you whether to really pass some pointer or some casted integer. In fact, PTHREAD_CANCELED is actually ((void *) -1) pthread_join (thread, void **retval_join) pthread_exit (void *retval_exit) On exit, *retval_join = retval_exit; I tried a sample program as per your suggestion. int ret1; pthread_join (thread, (void **)ret1); int ret2 = 1; pthread_exit (ret2); After pthread_join, ret1 has address of ret2 instead of its value. i.e, this led to ret1 = ret2; Please correct me if I am missing something. Had you used pthread_exit((void*)ret2) or better pthread_exit((void*)1), ret1 would have the integer 1. This seems to be the standard practice. If you really want to pass a pointer to pthread_exit(), make sure that the thread that called pthread_join() will be able to access such memory. Too much hassle, that is why most people just cast integers if they just need integer returns. It is up to you but I feel that the code gets unnecessarily complicated by returning a real pointer from the upcall thread. Okay. Thanks for sharing. I shall fix it as part of the improvements which we expect to do soon. Thanks, Soumya Regards, Malahal. -- ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
Re: [Nfs-ganesha-devel] Request to merge patches to V2.2-stable branch
I didn't see GLUSTERFSAL_UP_Thread() returning anything other than NULL/0. I think passing NULL to pthread_join is even cleaner! 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). 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? -- ___ 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
[Nfs-ganesha-devel] Request to merge patches to V2.2-stable branch
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 c4f33d6 - FSAL_GLUSTER : Improvements in acl feature b1df525 - FSAL_GLUSTER: Stop polling upcall events if not supported Thanks, Soumya -- ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
Re: [Nfs-ganesha-devel] Request to merge patches to V2.2-stable branch
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). 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? -- ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
