Differential D21206 Diff 60639 lib/libc/stdlib/fdwalk.c

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/sysctl.h>
				#include <sys/user.h>
				#include <stdlib.h>
				#include <unistd.h>

				/*
				* XXX: We should add a new sysctl to get just the list of file descriptors,
				* instead of the full properties. We do too much work to be thrown away.
				*/
				int
				fdwalk(int (cb)(void , int), void *cbd)
				{
				int mib[4];
				int error;
				size_t len;
				char buf, bp, *eb;
				struct kinfo_file *kf;
				int retries = 5; /* Arbitrary retry count. */
				kibUnsubmitted Done Inline Actions No initialization in declaration. kib: No initialization in declaration.

				len = 0;
				mib[0] = CTL_KERN;
				mib[1] = KERN_PROC;
				mib[2] = KERN_PROC_FILEDESC;
				kibUnsubmitted Done Inline Actions `if (error == -1)` kib: `if (error == -1)`
				mjgAuthorUnsubmitted Not Done Inline Actions You 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.
				mib[3] = getpid();

				kibUnsubmitted Done Inline Actions This 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…
				buf = NULL;

				/*
				* Try a few times to get a stable buffer. The buffer size may change
				* if file descriptors are being created in other threads.
				kibUnsubmitted Not Done Inline Actions I 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…
				mjgAuthorUnsubmitted Done Inline Actions It 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…
				kibUnsubmitted Not Done Inline Actions Add 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.
				*/
				for (; retries > 0; --retries) {
				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;
				mjgAuthorUnsubmitted Not Done Inline Actions this 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…
				rozhuk.im-gmail.comUnsubmitted Not Done Inline Actions Why 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…
				buf = reallocf(buf, len);
				if (buf == NULL)
				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);
				}
				mjgAuthorUnsubmitted Not Done Inline Actions I 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.
				bp = buf;
				eb = buf + len;
				while (bp < eb) {
				kf = (struct kinfo_file *)(uintptr_t)bp;
				kibUnsubmitted Not Done Inline Actions I 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.
				if (kf->kf_structsize == 0)
				break;
				error = cb(cbd, kf->kf_fd);
				if (error != 0)
				break;
				bp += kf->kf_structsize;
				}
				free(buf);

				return (error);
				mjgAuthorUnsubmitted Not Done Inline Actions Is 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…
				}