Changeset View
Standalone View
lib/libc/stdlib/fdwalk.c
- This file was added.
/*- | |||||
* SPDX-License-Identifier: BSD-2-Clause | |||||
* | |||||
* Copyright (c) 2019 Justin Hibbits | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND | |||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | |||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE | |||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | |||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | |||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
*/ | |||||
#include <sys/param.h> | |||||
#include <sys/filedesc.h> | |||||
#include <sys/sysctl.h> | |||||
#include <errno.h> | |||||
#include <stdlib.h> | |||||
#include <unistd.h> | |||||
#define FD(slot, bit) ((slot * sizeof(NDSLOTTYPE) * NBBY) + bit) | |||||
int | |||||
fdwalk(int (*cb)(void *, int), void *cbd) | |||||
{ | |||||
int mib[4]; | |||||
int error; | |||||
size_t len, i, j; | |||||
NDSLOTTYPE *buf, tmp; | |||||
int retries = 5; /* Arbitrary retry count. */ | |||||
len = 0; | |||||
mib[0] = CTL_KERN; | |||||
kib: No initialization in declaration. | |||||
mib[1] = KERN_PROC; | |||||
mib[2] = KERN_PROC_FDMAP; | |||||
mib[3] = getpid(); | |||||
Done Inline Actionsif (error == -1) kib: `if (error == -1)` | |||||
Not Done Inline ActionsYou can use 0 to signify the current process and avoid getpid altogether. mjg: You can use 0 to signify the current process and avoid getpid altogether. | |||||
buf = NULL; | |||||
Done Inline ActionsThis might be too strict in mt environment. We typically allocate more space than reported, to allow the attempt to succeed even if other thread opened a new file descriptor between query and fetch. More, sometimes we do not give up on the fetch failure but retry several times. kib: This might be too strict in mt environment. We typically allocate more space than reported, to… | |||||
/* | |||||
* Try a few times to get a stable buffer. The buffer size may change | |||||
* if file descriptors are being created in other threads. | |||||
*/ | |||||
for (; retries > 0; --retries) { | |||||
Not Done Inline ActionsI believe that the formal contract for alloca(3) is that the stack space is only freed on return from the function, not on the termination of the block scope. In other words, this loop allows for unbound stack allocation if raced. kib: I believe that the formal contract for alloca(3) is that the stack space is only freed on… | |||||
Done Inline ActionsIt can allocate multiple buffers, but each one is twice as big as the previous and the biggest you can allocate is loosely bounded by your fd limit. For more than two buffers to be allocated the code would effectively have to explicitly dup a fd outside of the previous range and keep doing it while winning the race for more buffers to be allocated. In other words this can use more memory only if the application is racing with itself on purpose and even then there is an upper bound to it. mjg: It can allocate multiple buffers, but each one is twice as big as the previous and the biggest… | |||||
Not Done Inline ActionsAdd a comment note that this code relies on the kernel returning specific values for newlen. kib: Add a comment note that this code relies on the kernel returning specific values for newlen. | |||||
error = sysctl(mib, nitems(mib), NULL, &len, NULL, 0); | |||||
if (error == -1) | |||||
return (-1); | |||||
/* | |||||
* Add some headroom in case more descriptors are added before | |||||
* the next call. | |||||
*/ | |||||
len = len * 4 / 3; | |||||
buf = reallocf(buf, len); | |||||
if (buf == NULL) | |||||
Not Done Inline Actionsthis formula looks like a leftover from previous attempt. sizes are guaranteed to be power of 2 times slot size. Also note my previous remark about calling this with a typically big enough buffer first and only retrying if needed. Each time sysctl can return the expected size so there is no need for manual calculation. Note the map is very rarely resized. mjg: this formula looks like a leftover from previous attempt. sizes are guaranteed to be power of 2… | |||||
Not Done Inline ActionsWhy not sysctl() return ENOMEM if buf size is lowr than needed? I create bug report for same behavior: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240573 rozhuk.im-gmail.com: Why not sysctl() return ENOMEM if buf size is lowr than needed?
I create bug report for same… | |||||
return (-1); | |||||
error = sysctl(mib, nitems(mib), buf, &len, NULL, 0); | |||||
if (error == 0) | |||||
break; | |||||
if (errno != ENOMEM) { | |||||
free(buf); | |||||
return (-1); | |||||
} | |||||
} | |||||
if (retries == 0) { | |||||
free(buf); | |||||
return (-1); | |||||
} | |||||
/* | |||||
* Go through the full file list. The fdmap is an integral multiple of | |||||
Not Done Inline ActionsI don't think the retry logic is useful. Instead you can pass a "typically big enough" buffer. Should it be not big enough, sysctl can return the real size along with ENOMEM in one call. Then you retry once with the new size. This avoids the initial call to get the size in the common case and retrying like in the above is very unlikely to be of concern for real-world programs - the bitmap itself would have to grow between calls. mjg: I don't think the retry logic is useful. Instead you can pass a "typically big enough" buffer. | |||||
* sizeof(NDSLOTTYPE). | |||||
*/ | |||||
len = howmany(len, sizeof(NDSLOTTYPE)); | |||||
for (i = 0; i < len; i++) { | |||||
Not Done Inline ActionsI do not think there is a need to cast through uinptr_t. kib: I do not think there is a need to cast through uinptr_t. | |||||
/* | |||||
* Iterate over each bit in the slot, short-circuting when there | |||||
* are no more file descriptors in use in this slot. | |||||
*/ | |||||
for (j = 0, tmp = buf[i]; | |||||
j < NBBY * sizeof(NDSLOTTYPE) && tmp != 0; | |||||
j++, tmp >>= 1) { | |||||
if (tmp & 1) { | |||||
error = cb(cbd, FD(i, j)); | |||||
Not Done Inline ActionsIs this testing tmp twice? I think it would be nicer to put this stuff inside the loop and 'continue' to skip. mjg: Is this testing tmp twice? I think it would be nicer to put this stuff inside the loop and… | |||||
if (error != 0) | |||||
goto done; | |||||
} | |||||
} | |||||
} | |||||
done: | |||||
free(buf); | |||||
return (error); | |||||
} |
No initialization in declaration.