On 05/05/2016 12:43 PM, Kornievskaia, Olga wrote: > >> On May 5, 2016, at 12:40 PM, Steve Dickson <SteveD@xxxxxxxxxx> wrote: >> >> >> >> On 05/05/2016 11:40 AM, Kornievskaia, Olga wrote: >>> >>>> On May 5, 2016, at 11:35 AM, Kornievskaia, Olga <Olga.Kornievskaia@xxxxxxxxxx <mailto:Olga.Kornievskaia@xxxxxxxxxx>> wrote: >>>> >>>>> >>>>> On May 5, 2016, at 10:17 AM, Steve Dickson <SteveD@xxxxxxxxxx <mailto:SteveD@xxxxxxxxxx>> wrote: >>>>> >>>>> Hello, >>>>> >>>>> On 05/04/2016 10:15 AM, Kornievskaia, Olga wrote: >>>>>> I’m having a hard time picturing what you have in mind. I don’t see how you can allocate, >>>>>> populate and free in one routine. The logic is we allocate memory, then read the upcall >>>>>> and only then we create the thread. If either upcall or creation of the thread >>>>>> fails we need to free the allocated memory. >>>>> Something like this: >>>>> >>>>> gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data) >>>>> { >>>>> struct clnt_info *clp = data; >>>>> - pthread_t th; >>>>> - int ret; >>>>> + struct clnt_upcall_info *info; >>>>> >>>>> - ret = pthread_create(&th, NULL, (void *)handle_gssd_upcall, >>>>> - (void *)clp); >>>>> - if (ret != 0) { >>>>> - printerr(0, "ERROR: pthread_create failed: ret %d: %s\n", >>>>> - ret, strerror(errno)); >>>>> + info = alloc_upcall_info(clp); >>>>> + if (info == NULL) >>>>> + return; >>>>> + >>>>> + info->lbuflen = read(clp->gssd_fd, info->lbuf, sizeof(info->lbuf)); >>>>> + if (info->lbuflen <= 0 || info->lbuf[info->lbuflen-1] != '\n') { >>>>> + printerr(0, "WARNING: %s: failed reading request\n", __func__); >>>>> + free(info); >>>>> return; >>>>> } >>>>> - wait_for_child_and_detach(th); >>>>> + info->lbuf[info->lbuflen-1] = 0; >>>>> + >>>>> + start_upcall_thread(handle_gssd_upcall, info); >>>>> + >>>>> + free(info); >>>>> + return; >>>> >>>> Gotcha! Sounds good! You can make the modifications. Or just in case since you are traveling, >>> I’ll just send another patch. Please also add any other lines like Modified-by or Signed-off-by that are need. >>> >>> I’m sorry I spoke too soon. “info” is allocated and given to the thread >>> to process so the main thread can’t free it. >> I'm curious as to why? The main thread allocated it, is handing allocated memory >> to a thread something special? What am I missing? > > The main thread allocates memory, gives it to the the child thread and goes > to the next libevent. We detach the thread so if we free the memory in the main > thread, the child thread would segfault. Ah... that is the part I missed... You are right... That's what I get for not testing... Let's go with the original patch and move on... steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
