This reverts commit f5a065a.

"lib/test-fork-safe-execvpe.sh" now works around Alpine Linux / BusyBox
limitations; reenable the test in the Alpine Linux containers.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

…ations

Whenever we expect our "bin/f" and "sh/f" files to execute (and to
succeed), provide "expr" and "sh" as argv[0], respectively. This is to
work around BusyBox, where the central busybox binary selects its behavior
based on argv[0]. See e.g.
<https://bugs.busybox.net/show_bug.cgi?id=15481>.

Also cope with "realpath" (from BusyBox) not accepting the "--"
end-of-options argument separator. See
<https://bugs.busybox.net/show_bug.cgi?id=15466>.

This patch is worth viewing with "git show --color-words" as well.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

It turns out that we'll need to put the generality of our
"test-fork-safe-execvpe" binary executable to use, in that it takes
separate "program-to-exec" and "argv0" (for the program-to-exec)
arguments.

Currently, the "run" function duplicates $2 to both "program-to-exec" and
"argv0", for "test-fork-safe-execvpe". Remove the duplication (expect the
caller to provide separate $2 and $3 arguments, respectively) and rename
"run" to "run0". At the same time, reimplement "run" as a simple wrapper
(i.e., with just the duplication) around "run0".

This patch is worth viewing with "git show --color-words" as well.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Alpine Linux provides the "expr" utility (under pathname "/usr/bin/expr")
in one of two forms: (1) as a symlink to the "/bin/busybox" binary
executable, when the "coreutils" package is not installed, (2) as a
symlink to the "coreutils" binary executable, otherwise. Both linked-to
binary executables key their behavior off the name under which they are
invoked (that is, argv0). Unfortunately, this breaks
"lib/test-fork-safe-execvpe.sh": that test case copies the "expr" utility
to a file called "bin/f", and actually expects "bin/f" to work. Both
"busybox" and "coreutils" croak when invoked as "f".

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

This reverts commit b29ff42.

Turns out the limitation that "realpath" does not recognize the "--"
end-of-options argument separator is just one kind of special sauce that
Alpine Linux / BusyBox gift to the world. I'm going to disable
"lib/test-fork-safe-execvpe.sh" on Alpine Linux altogether (Alpine Linux
is unsalvageable) in a subsequent patch; for now, restore the "realpath"
invocation to its right form.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Recent gcc in CI whines about attribute placement; try to fix it by
placing the attribute as we did it in commit 4a7f07a ("lib/utils:
introduce xwritel() as a more robust and convenient write()", 2023-03-16).

Fixes: aa696c0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Not all realpath variants recognize the "--" end-of-options argument
separator. Remove the "--" from "lib/test-fork-safe-execvpe.sh".

Fixes: 0b7172b
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Don't try to test async-signal-safety, but strive to exercise as many as
possible paths through nbd_internal_execvpe_init() and
nbd_internal_fork_safe_execvpe().

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230319094139.285312-3-lersek@redhat.com>

execvp() [01] is powerful:

- it performs PATH search [02] if necessary,

- it falls back to executing the file with the shell as a shell script in
  case the underlying execv() or execve() fails with ENOEXEC.

However, execvp() is not async-signal-safe [03], and so we shouldn't call
it in a child process forked from a multi-threaded parent process [04],
which the libnbd client application may well be.

Introduce three APIs for safely emulating execvp() with execve():

- nbd_internal_execvpe_init(): to be called in the parent process, before
  fork(). Allocate and format candidate pathnames for execution with
  execve(), based on PATH. Allocate a buffer for the longer command line
  that the shell script fallback needs.

- nbd_internal_fork_safe_execvpe(): to be called in the child process. Try
  the candidates formatted previously in sequence with execve(), as long
  as execve() fails with errors related to pathname resolution /
  inode-level file type / execution permission. Stop iterating on fatal
  errors; if the fatal error is ENOEXEC, activate the shell script
  fallback. As a small added feature, take the environment from an
  explicit parameter (effectively imitating the GNU-specific execvpe()
  function -- cf. commit dc64ac5, "states: Use execvp instead of
  execvpe", 2019-11-15).

- nbd_internal_execvpe_uninit(): to be called in the parent process, after
  fork(). Release the resources allocated with
  nbd_internal_execvpe_init().

The main idea with the above distribution of responsibilities is that any
pre-scanning of PATH -- which was suggested by Eric Blake -- should not
include activities that are performed by execve() anyway. I've considered
and abandoned two approaches:

- During scanning (i.e., in nbd_internal_execvpe_init()), check each
  candidate with access(X_OK) [05].

  I dropped this because the informative APPLICATION USAGE section of the
  same specification [06] advises against it, saying "[a]n application
  should instead attempt the action itself and handle the [EACCES] error
  that occurs if the file is not accessible".

- During scanning, open each candidate with O_EXEC [07] until one open()
  succeeds. Save the sole file descriptor for
  nbd_internal_fork_safe_execvpe(). In the latter, call fexecve() [08],
  which is free of all error conditions that necessitate iteration over
  candidates. Still implement the ENOEXEC (shell script) fallback.

  I dropped this because (a) fexecve() is a quite recent interface
  (highlighted by Eric); (b) for an application to open a file for
  execution with fexecve(), it's more robust to pass O_EXEC to open() than
  not to pass it, per APPLICATION USAGE [09], but on Linux/glibc, O_EXEC
  does not seem supported, only O_PATH does [10].

Thus the chosen approach -- pre-generate filenames -- contains a small
TOCTTOU race (highlighted by Eric) after all, but it should be harmless.

Implementation-defined details:

- If PATH searching is necessary, but PATH is absent [01], then fall back
  to confstr(_CS_PATH) [11], similarly to Linux/glibc execvpe() [12].

  If PATH is set but empty ("set to null") [02], or PATH is unset and
  confstr(_CS_PATH) fails or returns no information or returns the empty
  string, we fail the initial scanning (!) with ENOENT.

Details chosen for unspecified behavior:

- Hardwire "/bin/sh" as the shell's pathname [01].

[01] https://pubs.opengroup.org/onlinepubs/9699919799/functions/execvp.html
[02] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
[03] https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03_03
[04] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html
[05] https://pubs.opengroup.org/onlinepubs/9699919799/functions/access.html
[06] https://pubs.opengroup.org/onlinepubs/9699919799/functions/access.html#tag_16_09_07
[07] https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
[08] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fexecve.html
[09] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fexecve.html#tag_16_111_07
[10] https://man7.org/linux/man-pages/man2/open.2.html
[11] https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html
[12] https://man7.org/linux/man-pages/man3/execvpe.3.html

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20230319094139.285312-2-lersek@redhat.com>

Not all platforms provide pthread_getspecific() and friends in libc.

Fixes: c7d02b4 ("lib/utils: add unit test for async-signal-safe assert()")

Don't try to test async-signal-safety, only that
NBD_INTERNAL_FORK_SAFE_ASSERT() works similarly to assert():

- it prints diagnostics to stderr,

- it calls abort().

Some unfortunate gymnastics are necessary to avoid littering the system
with unwanted core dumps.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230315110157.357958-4-lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

Add an assert() variant that we may call between fork() and exec*().

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230315110157.357958-3-lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

While nbd_internal_fork_safe_perror() must indeed call write(), and
arguably justifiedly ignores the return value of write(), we can still
make the write operations slightly more robust and convenient. Let's do
that by introducing xwritel():

- Let the caller pass a list of NUL-terminated strings, via stdarg /
  ellipsis notation in the parameter list.

- Handle partial writes.

- Cope with EINTR and EAGAIN errors. (A busy loop around EAGAIN on a
  non-blocking file is not great in the general case, but it's good enough
  for writing out diagnostics before giving up.)

- In the common case, handle an nbd_internal_fork_safe_perror() call with
  a single xwritel() -> writev() call chain, rather than with four
  separate write() calls. In practice, this tends to make the error
  message written to a regular file contiguous, even if other threads are
  writing to the same file. Multiple separate write() calls tend to
  interleave chunks of data from different threads.

As a side bonus, remove the path in nbd_internal_fork_safe_perror() where
at least one of the first two write() syscalls fails, and overwrites
"errno", before we get to formatting the error string from "errno".

Thanks to Eric Blake for helping me understand the scope of Austin Group
bug reports.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230315110157.357958-2-lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[lersek@redhat.com: add __attribute__ ((sentinel)), suggested by Eric]

…ynch

In some cases user programs don't display the actual error from NBD.
Sometimes that's because it is hard, eg. in the callbacks in nbdcopy
where we only have the errno value.

This adds debug logging of the actual error string when we enter the
%DEAD state to make sure it is not lost.

When a highly multi-threaded program such as nbdcopy encounters an
error, there is a race condition in the library which can cause an
assertion failure and thus a core dump:

(1) An error occurs on one of the threads.  nbdcopy calls exit(3).

(2) In lib/errors.c, the destructor calls pthread_key_delete.

(3) Another thread which is still running also encounters an error,
and inside libnbd the library calls set_error().

(4) The call to set_error() calls pthread_getspecific which returns
NULL (since the key has already been destroyed in step (2)), and this
causes us to call pthread_setspecific which returns EINVAL because
glibc is able to detect invalid use of a pthread_key_t after it has
been destroyed.  In any case, the error message is lost, and any
subsequent call to nbd_get_error() will return NULL.

(5) We enter the %DEAD state, where there is an assertion that
nbd_get_error() is not NULL.  This assertion is supposed to be for
checking that the code called set_error() before entering the %DEAD
state.  Although we did call set_error(), the error message was lost
at step (4) and nbd_get_error() will always return NULL.  This
assertion failure causes a crash.

There aren't any good ways to fix this, so I chose the same method as
used by libvirt in this commit:

https://gitlab.com/libvirt/libvirt/-/commit/8e44e5593eb9b89fbc0b54fde15f130707a0d81e

(a) Use '-z nodelete' to prevent the library from being unloaded on
dlclose().

(b) Do not call pthread_key_destroy (thus leaking the key).

(c) When threads exit they are still able to call free_errors_key
because it is still present in memory.

Thanks: Daniel P. Berrangé, Eric Blake
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

We never expect this to fail, but did not check for failure.  At least
print something to stderr if this happens.  This is still unexpected,
so if it happens we should investigate.

Refactor the code which creates and destroys the pthread key storing
the per-thread error.

In set_error, if asprintf fails, collect the format string as the
error message as a last resort.

We've been advised to state Red Hat's Copyright as

  Copyright Red Hat

In order to collect our current copyright notices, I've used the following
"loose options" for git-grep (bash syntax):

  loose_options=(-i -e copyright --and -e 'red.?hat')

Namely, the command

  git grep -h "${loose_options[@]}"

produces 464 matches (lines) in libnbd @ d169661, and 861 matches
(lines) in nbdkit @ e36cfb6fa4f8.

Lest we replace strings too broadly, define the "strict pattern" as
follows (note that we assume LC_ALL=en_US.UTF-8):

  year='20[0-2][0-9]'
  year_range="$year(-$year)?"
  year_range_list="$year_range(, $year_range)*"
  year_range_list_opt="( $year_range_list)?"
  c_sym='(\(C\)|©)'
  company='Red Hat,? Inc\.'
  strict_pattern="[Cc]opyright $c_sym$year_range_list_opt $company"

The command

  git grep -h "${loose_options[@]}" \
  | grep -E -v -- "$strict_pattern"

produces zero lines in each of nbdkit and libnbd, meaning that the "strict
pattern" covers all matches from the "loose options".

Assuming that our filenames do not contain newline characters, replace the
copyright notices with the following command:

  git grep -l "${loose_options[@]}" \
  | sponge \
  | tr '\n' '\0' \
  | xargs -0 -r -- sed -i -E -e "s/$strict_pattern/Copyright Red Hat/" --

The resultant diffstat in each project shows that all notices have been
replaced (note that some files contain multiple notices, therefore the
number of files modified is less than the number of lines modified).

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

…port]

Original commit message:

Thanks: Eric Blake

Porting notes:

We already have the nesting fix in the "common/include/minmax.h" header
file, from nbdkit commit 9f462af12e3b ("common/include: Fix MIN and MAX
macros so they can be nested", 2022-02-22), via libnbd commit
2f25695. However, that port didn't include the update to the test
case. Do it now.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from nbdkit commit 9f462af12e3b8b9840435a266d7e6e7d45ed70bf)
Message-Id: <20230301114027.336230-7-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>

Original commit message:

Duplicate a vector, optionally in-place.

Porting notes: this port of nbdkit commit 78b72746e547 ("vector: Add
vector_duplicate function", 2021-08-22) includes:

- The renaming of vector fields from nbdkit commit 0b0eece73f04
  ("common/utils/vector: Rename `size` to `len`", 2021-11-07); otherwise,
  the patch wouldn't compile.

- The effect of libnbd commit 8fb8ffb ("build: Silence some cppcheck
  warnings", 2022-11-02), which added "__attribute__((__unused__))" to the
  other vector APIs.

- The effect of libnbd commit b5101fb ("use space consistently in
  function and function-like macro invocations", 2023-02-22), which
  inserted a space between "__attribute__" and "((__unused__))".

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from nbdkit commit 78b72746e547150290c2b0f2fe3ad5fe69817904)
Message-Id: <20230301114027.336230-6-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>

Original (nbdkit) commit message:

While we support compilation with CFLAGS=-DNDEBUG for the brave user
desiring to avoid assertion overhead, this causes the tests to be less
powerful (the tests "fail open" for anything where we relied on an
assert to validate nbdkit was behaving correctly; and this would be
even worse if we had side effects in a test assert, such as we just
fixed in d5cedf20).  Fortunately, a quick test of './configure
CFLAGS=-DNDEBUG' didn't hit any test failures, but it seems better to
just ensure that assertions always work in our tests, even when they
are turned off for the main binary.

Porting notes:

This is a partial port. At this point, the test cases we have under
common/ actually reflect our common/ header file usage (meaning that, if
nbdkit has a test case for a header file we use, then we also have that
test case). However, test case *content* remains different at this point;
one piece is nbdkit commit 0b6d62668584 ("tests: Avoid odd test behavior
under NDEBUG", 2020-07-07). Port those parts of it that modify such test
cases that libnbd does have, at this point. Note that the nbdkit commit
modifies "test-ispowerof2.c" as well, but we have ported that particular
NDEBUG addition earlier (in libnbd commit 6cc5123, "common/include:
Copy test-ispowerof2.c from nbdkit", 2022-08-25).

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from nbdkit commit 0b6d626685846d41ea113a394cb37e321c63e8bf)
Message-Id: <20230301114027.336230-5-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>

Complete porting nbdkit commit 2bc1f3383e00 ("common: include: Define
bswap_16, bswap_32 and bswap_64.", 2020-05-30). Libnbd commit def468c
("common/include/byte-swapping.h: Synchronize with nbdkit.", 2020-10-27)
had ported nbdkit commit 2bc1f3383e00 earlier, but it contains the
following omissions / mistakes:

- In the header file, it starts using the WORDS_BIGENDIAN macro, but it
  does not port the AC_C_BIGENDIAN stanza from nbdkit's "configure.ac".

- It does not port the test case extension.

Fill those gaps now.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from nbdkit commit 2bc1f3383e00bd5bbe2e1bb911ab56e1e42c4db3)
Message-Id: <20230301114027.336230-4-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>

Partially port nbdkit commit b55d85696172 ("common/include: Add further
unit tests.", 2019-01-09) to libnbd. In libnbd, we already use the header
"minmax.h", but we don't have the matching test case for it.

Porting notes:

- nbdkit commit b55d85696172 also introduces "test-nextnonzero". In
  libnbd, we don't have "nextnonzero.h" for the time being, so the test
  case is not ported either.

- In the test program being ported, update the (C) notice. Extend the year
  range to 2023, plus remove the "All rights reserved" line (to be
  consistent with the other test cases in libnbd).

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from nbdkit commit b55d85696172f9a54e7a5688897821d5f72237b4)
Message-Id: <20230301114027.336230-3-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>

Partially port nbdkit commit 58e7b0e104d5 ("common/include: Add unit
tests.", 2018-12-29) to libnbd. In libnbd, we already use the headers
"byte-swapping.h", "isaligned.h" and "iszero.h", but we don't have the
matching test cases for them.

Porting notes:

- nbdkit commit 58e7b0e104d5 also introduces "test-current-dir-name". In
  libnbd, we don't have "get-current-dir-name.h" for the time being, so
  the test case is not ported either.

- In libnbd, we have "ispowerof2" (both header and test case) from
  earlier, so porting that part of nbdkit commit 58e7b0e104d5 is not
  necessary.

- In the three test programs being ported, update the (C) notice. Extend
  the year range to 2023, plus remove the "All rights reserved" line (to
  be consistent with the other test cases in libnbd).

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from nbdkit commit 58e7b0e104d57d767d11aacab7d7ab82f0d4a98b)
Message-Id: <20230301114027.336230-2-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>

Convert the 11 string_vector_(iter(free) + reset()) call sites mentioned
previously to string_vector_empty().

Note that the converted code performs more cleanup steps in some cases
than strictly necessary, but the extra work is harmless, and arguably
beneficial for clarity / consistency.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230226114529.613013-6-lersek@redhat.com>

The "name##_iter" function is used 11 times in libnbd; in all those cases,
"name" is "string_vector", and the "f" callback is "free":

  string_vector_iter (..., (void *) free);

Casting "free" to (void*) is ugly. (Well-defined by POSIX, but still.)

Furthermore, in all 11 cases, the freeing of the vector's strings is
immediately followed by the release of the vector's array-of-pointers too.
(This additional step is not expressed consistently across libnbd: it's
sometimes spelled as free(vec.ptr), sometimes as
string_vector_reset(&vec).)

Introduce "name##_empty", which performs both steps at the same time.

Keep the generic "name##_iter" function definition, as we'll want to synch
this patch to nbdkit, and in nbdkit, "name##_iter" has other uses as well.

Expose the "name##_empty" function definition with a new, separate macro:
ADD_VECTOR_EMPTY_METHOD(). The existent DEFINE_VECTOR_TYPE() macro permits
such element types that are not pointers, or are pointers to const- and/or
volatile-qualified objects. Whereas "name##_empty" requires that the
elements be pointers to dynamically allocated, non-const, non-volatile
objects.

Add DEFINE_POINTER_VECTOR_TYPE() that expands to both DEFINE_VECTOR_TYPE()
and the additive ADD_VECTOR_EMPTY_METHOD().

(

For example, after

  typedef char foobar[5];

the following compiles (as expected):

  DEFINE_VECTOR_TYPE (foobar_vector, foobar);

and the following fails to build (as expected):

  DEFINE_POINTER_VECTOR_TYPE (foobar_vector_bad, foobar);

with the diagnostics including

> In function ‘foobar_vector_bad_empty’:
> error: size of array ‘_vector_contains_pointers1’ is negative

)

Switch "string_vector" to DEFINE_POINTER_VECTOR_TYPE(). The 11
string_vector_iter() call sites continue working; they will be converted
to string_vector_empty() in a subsequent patch.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230226114529.613013-5-lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

Currently, a semicolon after a DEFINE_VECTOR_TYPE(...) macro invocation is
not needed, nor does our documentation in "common/utils/vector.h"
prescribe one. Furthermore, it breaks ISO C, which gcc reports with
"-Wpedantic":

> warning: ISO C does not allow extra ‘;’ outside of a function
> [-Wpedantic]

Our current usage is inconsistent; a proper subset of all our
DEFINE_VECTOR_TYPE() invocations is succeeded by a semicolon. We could
remove these semicolons, but that doesn't play nicely with Emacs's C
language parser. Thus, force the semicolon instead.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230226114529.613013-4-lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

We already have two use cases for static assertions (and soon we'll have
yet another). Namely:

- STATIC_ASSERT_UNSIGNED_INT() in "checked-overflow.h". Here, we use our
  own trick, based on a negative-sized array typedef that's named with
  NBDKIT_UNIQUE_NAME.

- static_assert() in "test-array-size.c". This uses the C11 macro called
  static_assert() from <assert.h>, which wraps the C11 _Static_assert().
  This is not really great: our baseline is C99, not C11 (per commit
  762f7c9, "tests: Set minimum compiler to ISO C99.", 2021-04-08) --
  which is why the same assertions are repeated in the code as normal
  runtime assert() calls, in case static_assert() is not defined.

Factor out our own STATIC_ASSERT(), from STATIC_ASSERT_UNSIGNED_INT().

Put it to use in "test-array-size.c", replacing both the runtime assert()s
and the compile-time static_assert()s. Note that for the latter, in order
to remain consistent with STATIC_ASSERT_UNSIGNED_INT(), we no longer
provide the *complaint* that we want the compiler to emit upon assertion
failure, but an identifier that stands for the predicate that we *expect*.

When uncommenting the negative test case in "test-array-size.c", the
resultant wall of compiler diagnostics includes the following entry:

> test-array-size.c:83:39: error: size of array
> ‘_array_size_macro_is_applied_to_array13’ is negative

This patch will have to be ported to nbdkit. (IMO we can implement the
change in libnbd first -- the "common" subdir is meant to be common, so no
particular precedence should be assumed.)

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230226114529.613013-3-lersek@redhat.com>

Because the current definition of TYPE_IS_ARRAY() already contains a
negation, defining TYPE_IS_POINTER() in terms of TYPE_IS_ARRAY() would
lead to double negation, which I consider less than ideal style.
Therefore, introduce TYPE_IS_POINTER() from scratch, and rebase
TYPE_IS_ARRAY() on top of TYPE_IS_POINTER().

Suggested-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230226114529.613013-2-lersek@redhat.com>

Mention the correct library name in the exception, and make sure
__del__ can be invoked more than once.

Fixes: b7f8561 ("python: Add explicit h.close() method")