fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-02-19 01:50:48 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

all: merge branch 'th/nm-close'

Browse source 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1430
This commit is contained in:
Thomas Haller 2022-10-25 15:42:13 +02:00
commit ced829d8fd
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
9 changed files with 218 additions and 121 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
contrib/scripts/checkpatch.pl
View file

@ -206,6 +206,7 @@ complain ("Define setting properties with _nm_setting_property_define_direct_*()
complain ("Use nm_g_array_{index,first,last,index_p}() instead of g_array_index(), as it nm_assert()s for valid element size and out-of-bound access") if $line =~ /\bg_array_index\b/;
complain ("Use spaces instead of tabs") if $line =~ /\t/;
complain ("Prefer implementing private pointers via _NM_GET_PRIVATE() or _NM_GET_PRIVATE_PTR() (the latter, if the private data has an opqaue pointer in the header file)") if $line =~ /\b(g_type_class_add_private|G_TYPE_INSTANCE_GET_PRIVATE)\b/;
complain ("Don't use close()/g_close(). Instead, use nm_close() (or nm_close_with_error()).") if $line =~ /\b(close|g_close)\b *\(/;
complain ("Use nm_memdup() instead of g_memdup(). The latter has a size argument of type guint") if $line =~ /\bg_memdup\b/;
# Further on we process stuff without comments.

27
src/core/main-utils.c
View file

@ -18,6 +18,7 @@
#include "main-utils.h"
#include "NetworkManagerUtils.h"
#include "nm-config.h"
#include "libnm-glib-aux/nm-io-utils.h"
static gboolean
sighup_handler(gpointer user_data)
@ -76,30 +77,16 @@ nm_main_utils_setup_signals(GMainLoop *main_loop)
gboolean
nm_main_utils_write_pidfile(const char *pidfile)
{
char pid[16];
int fd;
int errsv;
gboolean success = FALSE;
gs_free_error GError *error = NULL;
char pid[16];
if ((fd = open(pidfile, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 00644)) < 0) {
errsv = errno;
fprintf(stderr, _("Opening %s failed: %s\n"), pidfile, nm_strerror_native(errsv));
nm_sprintf_buf(pid, "%lld", (long long) getpid());
if (!nm_utils_file_set_contents(pidfile, pid, -1, 00644, NULL, NULL, &error)) {
fprintf(stderr, _("Writing to %s failed: %s\n"), pidfile, error->message);
return FALSE;
}
g_snprintf(pid, sizeof(pid), "%d", getpid());
if (write(fd, pid, strlen(pid)) < 0) {
errsv = errno;
fprintf(stderr, _("Writing to %s failed: %s\n"), pidfile, nm_strerror_native(errsv));
} else
success = TRUE;
if (nm_close(fd)) {
errsv = errno;
fprintf(stderr, _("Closing %s failed: %s\n"), pidfile, nm_strerror_native(errsv));
}
return success;
return TRUE;
}
void

12
src/libnm-glib-aux/nm-io-utils.c
View file

@ -352,6 +352,7 @@ nm_utils_file_set_contents(const char *filename,
int errsv;
gssize s;
int fd;
int r;
g_return_val_if_fail(filename, FALSE);
g_return_val_if_fail(contents || !length, FALSE);
@ -415,7 +416,16 @@ nm_utils_file_set_contents(const char *filename,
tmp_name);
}
nm_close(fd);
r = nm_close_with_error(fd);
if (r < 0) {
errsv = NM_ERRNO_NATIVE(-r);
unlink(tmp_name);
return _get_contents_error(error,
errsv,
out_errsv,
"failed close() after writing file %s",
tmp_name);
}
if (rename(tmp_name, filename)) {
errsv = NM_ERRNO_NATIVE(errno);

16
src/libnm-glib-aux/nm-macros-internal.h
View file

@ -525,16 +525,22 @@ nm_str_realloc(char *str)
/*****************************************************************************/
/* redefine assertions to use g_assert*() */
#undef _nm_assert_call
#undef _nm_assert_call_not_reached
#define _nm_assert_call(cond) g_assert(cond)
#define _nm_assert_call_not_reached() g_assert_not_reached()
#undef _nm_assert_fail
#define _nm_assert_fail(msg) \
g_assertion_message_expr(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, msg)
#undef _NM_ASSERT_FAIL_ENABLED
#ifndef G_DISABLE_ASSERT
#define _NM_ASSERT_FAIL_ENABLED 1
#else
#define _NM_ASSERT_FAIL_ENABLED 0
#endif
/* Usage:
*
* if (NM_MORE_ASSERT_ONCE (5)) { extra_check (); }
*
* This will only run the check once, and only if NM_MORE_ASSERT is >= than
* This will only run the check once, and only if NM_MORE_ASSERTS is >= than
* more_assert_level.
*/
#define NM_MORE_ASSERT_ONCE(more_assert_level) \

3
src/libnm-glib-aux/nm-test-utils.h
View file

@ -85,6 +85,9 @@
#undef g_assertion_message_expr
#endif
#undef NDEBUG
#include <assert.h>
#include <arpa/inet.h>
#include <stdio.h>
#include <unistd.h>

3
src/libnm-glib-aux/tests/test-shared-general.c
View file

@ -78,6 +78,9 @@ test_gpid(void)
* the case. */
int_ptr = &pid;
g_assert_cmpint(*int_ptr, ==, 42);
/* also check how assert() works. */
assert(*int_ptr == 42);
}
/*****************************************************************************/

23
src/libnm-platform/nm-linux-platform.c
View file

@ -5608,12 +5608,14 @@ sysctl_set_internal(NMPlatform *platform,
const char *path,
const char *value)
{
int fd, tries;
int fd;
int tries;
gssize nwrote;
gssize len;
char *actual;
gs_free char *actual_free = NULL;
int errsv;
int r;
if (dirfd < 0) {
pathid = path;
@ -5707,18 +5709,13 @@ sysctl_set_internal(NMPlatform *platform,
_LOGE("sysctl: failed to set '%s' to '%s' after three attempts", path, value);
}
if (nwrote < len - 1) {
if (nm_close(fd) != 0) {
if (errsv != 0)
errno = errsv;
} else if (errsv != 0)
errno = errsv;
else
errno = EIO;
return FALSE;
}
if (nm_close(fd) != 0) {
/* errno is already properly set. */
r = nm_close_with_error(fd);
if (r < 0 || nwrote < len - 1) {
if (errsv == 0) {
/* propagate the error from nm_close_with_error(). */
errsv = (r < 0) ? -r : EIO;
}
errno = errsv;
return FALSE;
}

2
src/libnm-platform/wifi/nm-wifi-utils-wext.c
View file

@ -90,7 +90,7 @@ dispose(GObject *object)
{
NMWifiUtilsWext *wext = NM_WIFI_UTILS_WEXT(object);
wext->fd = nm_close(wext->fd);
nm_clear_fd(&wext->fd);
}
static gboolean

252
src/libnm-std-aux/nm-std-aux.h
View file

@ -20,6 +20,7 @@
#define _nm_used __attribute__((__used__))
#define _nm_pure __attribute__((__pure__))
#define _nm_const __attribute__((__const__))
#define _nm_noreturn __attribute__((__noreturn__))
#define _nm_warn_unused_result __attribute__((__warn_unused_result__))
#define _nm_printf(a, b) __attribute__((__format__(__printf__, a, b)))
#define _nm_align(s) __attribute__((__aligned__(s)))
@ -193,60 +194,88 @@ typedef uint64_t _nm_bitwise nm_be64_t;
#define NM_MORE_ASSERTS 0
#endif
#ifndef _nm_assert_call
#define _nm_assert_call(cond) assert(cond)
#define _nm_assert_call_not_reached() assert(0)
#if NM_MORE_ASSERTS == 0
/* The string with the assertion check and the function name blows up the
* binary size. In production mode, let's drop those, similar to
* g_assertion_message_expr.
*
* Note that <assert.h> can be included multiple times. We can thus
* not redefine __assert_fail(...). Instead, just redefine the name
* __assert_fail. */
_nm_noreturn static inline void
_nm_assert_fail_internal(const char *assertion,
const char *file,
unsigned int line,
const char *function)
{
__assert_fail("<dropped>", file, line, "<unknown-fcn>");
}
#define __assert_fail _nm_assert_fail_internal
#endif
#if NM_MORE_ASSERTS
#define nm_assert(cond) \
({ \
_nm_assert_call(cond); \
1; \
})
#define nm_assert_se(cond) \
({ \
if (NM_LIKELY(cond)) { \
; \
} else { \
_nm_assert_call(0 && (cond)); \
} \
1; \
})
#define nm_assert_not_reached() \
({ \
_nm_assert_call_not_reached(); \
1; \
})
#define _nm_assert_fail(msg) __assert_fail((msg), __FILE__, __LINE__, __func__)
#ifndef NDEBUG
#define _NM_ASSERT_FAIL_ENABLED 1
#else
#define nm_assert(cond) \
({ \
if (0) { \
if (cond) {} \
} \
1; \
})
#define nm_assert_se(cond) \
({ \
if (NM_LIKELY(cond)) { \
; \
} \
1; \
})
#define nm_assert_not_reached() ({ 1; })
#define _NM_ASSERT_FAIL_ENABLED 1
#endif
#define NM_MORE_ASSERTS_EFFECTIVE (_NM_ASSERT_FAIL_ENABLED ? NM_MORE_ASSERTS : 0)
#define nm_assert(cond) \
({ \
/* nm_assert() must do *nothing* of effect, except evaluating
* @cond (0 or 1 times).
*
* As such, nm_assert() is async-signal-safe (provided @cond is, and
* the assertion does not fail). */ \
if (NM_MORE_ASSERTS_EFFECTIVE == 0) { \
/* pass */ \
} else if NM_LIKELY (cond) { \
/* pass */ \
} else { \
_nm_assert_fail(#cond); \
} \
1; \
})
#define nm_assert_se(cond) \
({ \
/* nm_assert() must do *nothing* of effect, except evaluating
* @cond (exactly 1 times).
*
* As such, nm_assert() is async-signal-safe (provided @cond is, and
* the assertion does not fail). */ \
if NM_LIKELY (cond) { \
/* pass */ \
} else if (NM_MORE_ASSERTS_EFFECTIVE == 0) { \
/* pass */ \
} else { \
_nm_assert_fail(#cond); \
} \
1; \
})
#define nm_assert_not_reached() \
({ \
_nm_assert_fail("unreachable"); \
1; \
})
/* This is similar nm_assert_not_reached(), but it's supposed to be used only during
* development. Like _XXX_ comments, they can be used as a marker that something still
* needs to be done. */
#define XXX(msg) \
nm_assert(!"X" \
"XX error: " msg "")
#define XXX(msg) \
({ \
_nm_assert_fail("X" \
"XX error: " msg ""); \
1; \
})
#define nm_assert_unreachable_val(val) \
({ \
nm_assert_not_reached(); \
(val); \
#define nm_assert_unreachable_val(val) \
({ \
_nm_assert_fail("unreachable value " #val); \
(val); \
})
#define NM_STATIC_ASSERT(cond) static_assert(cond, "")
@ -928,38 +957,6 @@ _NM_IN_STRSET_EVAL_op_streq(const char *x1, const char *x)
/*****************************************************************************/
/**
* nm_close:
*
* Like close() but throws an assertion if the input fd is
* invalid. Closing an invalid fd is a programming error, so
* it's better to catch it early.
*/
static inline int
nm_close(int fd)
{
int r;
r = close(fd);
nm_assert(r != -1 || fd < 0 || errno != EBADF);
return r;
}
static inline bool
nm_clear_fd(int *p_fd)
{
int fd;
if (!p_fd || (fd = *p_fd) < 0)
return false;
*p_fd = -1;
nm_close(fd);
return true;
}
/*****************************************************************************/
/* Note: @value is only evaluated when *out_val is present.
* Thus,
* NM_SET_OUT (out_str, g_strdup ("hallo"));
@ -1037,13 +1034,106 @@ _nm_auto_protect_errno(const int *p_saved_errno)
/*****************************************************************************/
/**
* nm_close_with_error:
*
* Wrapper around close().
*
* This fails an nm_assert() for EBADF with a non-negative file descriptor. Trying
* to close an invalid file descriptor is always a serious bug. Never use close()
* directly, because we want to catch such bugs.
*
* This also suppresses any EINTR and pretends success. That is appropriate
* on Linux (but not necessarily on other POSIX systems).
*
* In no case is it appropriate to use @fd afterwards (or retry).
*
* This function returns 0 on success, or a negative errno value.
* On success, errno is undefined afterwards. On failure, errno is
* the same as the (negative) return value.
*
* In the common case, when you don't intend to handle the error from close(),
* prefer nm_close() over nm_close_with_error(). Never use close() directly.
*
* The function is also async-signal-safe (unless an assertion fails).
*
* Returns: 0 on success or the negative errno from close().
*/
static inline int
nm_close_with_error(int fd)
{
int r;
r = close(fd);
if (r != 0) {
int errsv = errno;
nm_assert(r == -1);
/* EBADF indicates a bug.
*
* - if fd is non-negative, this means the tracking of the descriptor
* got messed up. That's very bad, somebody closed a wrong FD or we
* might do so. On a multi threaded application, messing up the tracking
* of the file descriptor means we race against closing an unrelated FD.
* - if fd is negative, it may not be a bug but intentional. However, our callers
* are not supposed to call close() on a negative FD either. Assert
* against that too. */
nm_assert(errsv != EBADF);
if (errsv == EINTR) {
/* There isn't really much we can do about EINTR. On Linux, always this means
* the FD was closed. On some POSIX systems that may be different and retry
* would be appropriate.
*
* Whether there was any IO error is unknown. Assume not and signal success. */
return 0;
}
return -errsv;
}
return 0;
}
/**
* nm_close:
*
* Wrapper around nm_close_with_error(), which ignores any error and preserves the
* caller's errno.
*
* We usually don't care about errors from close, so this is usually preferable over
* nm_close_with_error(). Never use close() directly.
*
* Everything from nm_close_with_error() applies.
*/
static inline void
nm_close(int fd)
{
NM_AUTO_PROTECT_ERRNO(errsv);
nm_close_with_error(fd);
}
static inline bool
nm_clear_fd(int *p_fd)
{
int fd;
if (!p_fd || (fd = *p_fd) < 0)
return false;
*p_fd = -1;
nm_close(fd);
return true;
}
static inline void
_nm_auto_close(int *pfd)
{
if (*pfd >= 0) {
NM_AUTO_PROTECT_ERRNO(errsv);
(void) nm_close(*pfd);
}
if (*pfd >= 0)
nm_close(*pfd);
}
#define nm_auto_close nm_auto(_nm_auto_close)