From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id EC0B83858D20 for ; Mon, 15 May 2023 17:39:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EC0B83858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-ot1-x336.google.com with SMTP id 46e09a7af769-6ac966d3706so2742648a34.2 for ; Mon, 15 May 2023 10:39:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1684172398; x=1686764398; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=zyGogfh81d+kzXUcaimIm4yoUMCIp5zFZqDcNWL2h7g=; b=xRCnhhg3gZ2oCUhR6jSDYb0ouLQXabHlUy7yT1IkzK8YZ249xz/1N2QGHyDOqvDozt 57GHp3dEeVW805ld3KytOWcv/jg7v/aU57N4kgN+eNz5CpTVF/ooiEAUqi9/GMnvL4A4 tOJEX+XFvLnk2t5Rb3YvdSt7NO6sxs3bcgYnj9FGTaAJswFsGhHEp3OkmahKKEI60rHl nwsM4T1cRdng9M0c4jEJvtLANT0nQK8sxJpom/+1u91TOG+0fFapamhoVFqska+4cVCu 5oG3gF/zjRR9TSxjm63Dj4MOq7WhLV+XtCAEcRTIr2CRUiy0vI36/2Nz4ONtHcBk5UnY 7X4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684172398; x=1686764398; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zyGogfh81d+kzXUcaimIm4yoUMCIp5zFZqDcNWL2h7g=; b=ARQq+9fWAKiXG+1KVfR20mUt64senzNHGkQ2L8VKI8AYCBPB/ZSXqzW8wOlHmCNMSR wXs7nZFwtp04G9mr1KSOVYQ7djNULmhE6ZX/nxs6L5RoFkLRyklIdg9T5VSY3QS1dJQP KRrJ72lyaoIw1nbKOuR7F5OWJ+9bWVT1UK0yqz8FuE6JW9WSQeioCbZd+ReUq70jTSsU QLblAycCgPmKk/fAW073mTfUsPaaW3oBMKGsOEJFxBgE/wVivKz8Blzfty7Z/qYz/adY V+PoxKi5JO/XIcGhr9CPBEKeV5IpJ3LhV2YsHrLRL8WfPqkzC47BMrZ18R6xehOLBWUk 4SQg== X-Gm-Message-State: AC+VfDzb4C+RMIhBJiWTl732YLUhNoNUCXKhs8+k03s5RpPdhYDfrS0X Odrf8okc6FZ0vsL9iYqe0XYK8Q== X-Google-Smtp-Source: ACHHUZ6IchIL/NNJOhFUGqjAUZjM9Q+4CQ9nKDLm6RPFjuzCS00WShssGmaSYbcaLTbkP3kJgntB2w== X-Received: by 2002:a05:6830:1053:b0:6a6:2f86:978d with SMTP id b19-20020a056830105300b006a62f86978dmr12865662otp.12.1684172398238; Mon, 15 May 2023 10:39:58 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c0:7359:809b:29db:c429:1aa0? ([2804:1b3:a7c0:7359:809b:29db:c429:1aa0]) by smtp.gmail.com with ESMTPSA id l25-20020a0568301d7900b006adc88611basm1422455oti.67.2023.05.15.10.39.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 15 May 2023 10:39:57 -0700 (PDT) Message-ID: Date: Mon, 15 May 2023 14:39:55 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v2 3/3] linux: Add pidfd_getpid Content-Language: en-US To: Florian Weimer , Adhemerval Zanella via Libc-alpha Cc: Luca Boccassi References: <20230420142037.4063169-1-adhemerval.zanella@linaro.org> <20230420142037.4063169-4-adhemerval.zanella@linaro.org> <87r0rysomw.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <87r0rysomw.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-15.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 02/05/23 13:07, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> This interface allows to obtain the associated pid ID from the >> process file descriptor. It is done by parsing the procps fdinfo >> information. Its prototype is: >> >> pid_t pidfd_getpid (int fd) >> >> It returns the associated pid or -1 in case of an error and set the >> errno accordingly. The possible errno values are the smae from >> open, read, and close (used on procps parsing), along with: >> >> - EINVAL if the FP is negative (similar to fexecve). >> >> - EBADF if the FD does not have a PID associated of if the fdinfo >> fields contains a value larger than pid_t. >> >> - EREMOTE if the PID is in a separate namespace. >> >> - ESRCH if the process is already terminated. > > Could you add a manual entry for this? > > The documentation for EREMOTE (also in the comment in the code) should > probably say that this is returned if there is no PID to denote the > process in the current namespace. I assume that the PID is available > from an outer namespace (even across PID namespace boundaries), but not > necessarily in the other direction. Ok, I will update the patch with a manual entry. I will also add a testcase for EREMOTE. > >> diff --git a/sysdeps/unix/sysv/linux/pidfd_getpid.c b/sysdeps/unix/sysv/linux/pidfd_getpid.c >> new file mode 100644 >> index 0000000000..d0c7987791 >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/pidfd_getpid.c >> @@ -0,0 +1,94 @@ > >> +static bool >> +parse_fdinfo (const char *l, void *arg) >> +{ >> + enum { fieldlen = sizeof ("Pid:") - 1 }; >> + if (strncmp (l, "Pid:", fieldlen) != 0) >> + return true; >> + >> + l += fieldlen; >> + >> + char *endp; >> + long int n = strtol (l, &endp, 10); >> + if (l == endp || n > INT_MAX) >> + return true; > > Should this use strtoul? Otherwise I'm not sure the overflow check will > work on 32-bit. Ack. > > Rest of the implementation looks okay, but the kernel should really > provide an ioctl or similar for this. It would be the best option, although assuming procps support it is just a mater to correct parsing it (although such things is frequently subject to subtle bugs). > >> diff --git a/sysdeps/unix/sysv/linux/procutils.c b/sysdeps/unix/sysv/linux/procutils.c >> new file mode 100644 >> index 0000000000..4909afeae1 >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/procutils.c >> @@ -0,0 +1,99 @@ >> +/* Utilities functions to read/parse Linux procfs and sysfs. > >> +int >> +procutils_read_file (const char *filename, procutils_closure_t closure, >> + void *arg) >> +{ >> + enum { buffer_size = 1024 }; >> + char buffer[buffer_size]; >> + char *buffer_end = buffer + buffer_size; >> + char *cp = buffer_end; >> + char *re = buffer_end; >> + >> + int fd = __open64_nocancel (filename, O_RDONLY | O_CLOEXEC); >> + if (fd == -1) >> + return -1; >> + >> + char *l; >> + while ((l = next_line (fd, buffer, &cp, &re, buffer_end)) != NULL) >> + if (!closure (l, arg)) >> + break; >> + >> + __close_nocancel_nostatus (fd); >> + >> + return 0; >> +} > > Incomplete error reporting? Any read failure doesn't make it to the > caller. Ack, I will fix to report read errors. > > The callback interface will be easier to use if you have it return int > and return the callback return value if it is non-zero, I think. Ack. > >> diff --git a/sysdeps/unix/sysv/linux/tst-pidfd.c b/sysdeps/unix/sysv/linux/tst-pidfd.c >> index 64d8a2ef40..2a73328ef1 100644 >> --- a/sysdeps/unix/sysv/linux/tst-pidfd.c >> +++ b/sysdeps/unix/sysv/linux/tst-pidfd.c >> @@ -18,6 +18,7 @@ > > It would be nice to have a check for the EREMOTE case. > > Thanks, > Florian >