`GByteArray` is a bit odd in that it allows call-chaining. All the
instances of that were missing a `(transfer none)` annotation though.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
Although g_ptr_array_sort_with_data() could achieve the goal, the
wrapper functions should be expected there because that's the reason
why they are added.
`memcpy(NULL, ., n)` and `memcpy(., NULL, n)` are undefined behaviour,
even if *n* is zero.
When len is 0 here, callers are allowed to pass in null data, and
GPtrArray also does not guarantee to have allocated rarray->pdata yet.
Signed-off-by: Simon McVittie <smcv@collabora.com>
ptr_array_new(len, ., TRUE) ensures that there are at least len+1
elements in pdata, and that pdata[0] is null, but leaves the rest of
pdata uninitialized. After copying the array data into pdata[1] to
pdata[len-1] inclusive, we still need to make sure pdata[len] is a
null terminator.
Note that if len is 0, then pdata is not guaranteed to be non-null. If
it's null, then we can't add null-termination to it until its size
is updated.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2877
Signed-off-by: Simon McVittie <smcv@collabora.com>
Historically GPtrArray made possible to compare pointers of pointers values
that it holds, however this is inconvenient in most cases as it requires
wrapper functions and not friendly castings.
So, add two functions that allow to perform the comparisons between the
pointer values that a GPtrArray holds following the same syntax that we
share everywhere in the codebase.
It allows to create a GPtrArray from a null-terminated C array computing its
size and in case performing copies of the its values using the provided
GCopyFunc.
GPtrArray is a nice interface to handle pointer arrays, however if a classic
array needs to be converted into a GPtrArray is currently needed to manually
go through all its elements and do new allocations that could be avoided.
So add g_ptr_array_new_take() which steals the data from an array of
pointers and allows to manage it using the GPtrArray API.
GArray's g_array_append_val(), g_array_prened_val() and g_array_insert_val()
macros required an user to use literals to add a new value.
This could be inconvenient at times, but it's possible to avoid this with
recent compilers, in fact in case glib_typeof is defined we can take
advantage of it, to initialize a temporary variable to store the literal
value and pass its address to the actual function.
In both these cases, the static analyser (Coverity) was worrying that
the array `data`/`pdata` wasn’t allocated before an element was written
to. That was a false positive: all the necessary conditions are met in
both cases for `g_{ptr_,}array_maybe_expand()` to always allocate the
array.
But it makes things a bit easier for the analyser if we add an assertion
to double-check that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1474426, #1489512
`ptr_array_null_terminate()` only `NULL`-terminates the array if its
`null_terminated` flag is set; otherwise it’s a no-op.
Rename the function to `ptr_array_maybe_null_terminate()` to make that a
bit clearer, and make it consistent with `g_ptr_array_maybe_expand()`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Recent changes to `GPtrArray` and/or Coverity mean that Coverity is now
assuming that `g_ptr_array_free (my_array, TRUE)` can leak memory. This
is true in the case that `g_ptr_array_ref (my_array)` has been called
elsewhere, but Coverity never actually verifies that.
Very little (or no?) GLib code mixes `g_ptr_array_free()` with
`g_ptr_array_{ref,unref}()`, so this isn’t a problem in practice.
However, it has created a hundred or more false positives in Coverity
(as pointer arrays are widely used within GLib and GIO), which is a
complete pain.
Before taking the dramatic step of ditching Coverity due to its
atrocious false positive rate, let’s try changing the semantics of
`g_ptr_array_free()` only when running under Coverity.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The code was accidentally NULL-terminating the source array rather than
the copy.
This fixes commit ee247c0a2d.
Spotted by the `array-test` installed test in
https://gitlab.gnome.org/GNOME/glib/-/jobs/2047993.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
GArray supports a "zero_terminated" flag, but GPtrArray doesn't.
This is odd, because especially for a pointer array it makes sense
to have a %NULL sentinel. This would be for example useful to track
or construct a strv array with a GPtrArray.
As workaround for this missing feature you could use a GArray instead
(ugly) or to explicitly add the %NULL element. However the latter increases
the "len" of the array, which can be problematic if you want to still use
the GPtrArray for other purposes.
Add API for marking a GPtrArray as %NULL terminated. In that case, the
API will ensure that there is always a valid %NULL sentinel after the
array. Note that the API does not enforce that a %NULL terminated API
actually has any data allocated. That means, even with a %NULL terminated
array, pdata can still be %NULL (only if len is zero).
Add g_ptr_array_new_null_terminated() constructor. The null-terminated flag
cannot be cleared. Once the GPtrArray is flagged to be %NULL terminated, it
sticks. The purpose is that once a user checks whether a GPtrArray instance
is safe to be treated as a %NULL terminated array, the decision does
not need to be re-evaluated.
Also add a g_ptr_array_is_null_terminated(). That is useful because it
allows you to check whether a GPtrArray created by somebody else is safe
to use as a %NULL terminated array. Since there is no API to make an
array not %NULL terminated anymore, this is not error prone.
The new flag is tracked as a guint8 in GRealPtrArray. On common 64 bit
architectures this does not increase the size of the struct as it fits
in an existing hole. Note that this is not a bitfield because it's
probably more efficient to access the entire guint8. However, there is
still a 3 bytes hole (on common 32 and 64 architectures), so if we need
to add more flags in the future, we still have space for 24 bits,
despite the new flag not being a bitfield.
The biggest downside of the patch is the runtime overhead that most
operations now need to check whether %NULL termination is requested.
Includes some tweaks and additional tests by Philip Withnall.
https://gitlab.gnome.org/GNOME/glib/-/issues/353
Add SPDX license (but not copyright) headers to all files which follow a
certain pattern in their existing non-machine-readable header comment.
This commit was entirely generated using the command:
```
git ls-files glib/*.[ch] | xargs perl -0777 -pi -e 's/\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/\n \*\n \* SPDX-License-Identifier: LGPL-2.1-or-later\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/igs'
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
Integer overflows in size calculations of buffers (GArray and GPtrArray)
allow subsequent buffer overflows. This happens due to conversions
between gsize and guint.
Proof of concept demonstrations of the overflows can be found in issue
2578. They are not being added as unit tests as they require too much
memory to test.
This will affect `GArray`s which are 4GB in size, or `GPtrArray`s which
are 48GB in size.
Fixes: #2578
It might otherwise happen that the return value from g_nearest_pow()
does not fit into a guint, i.e. it might be G_MAXUINT + 1 if that fits
into a gsize.
glib.git/glib/garray.c: In function ‘g_array_new’:
glib.git/glib/garray.c:184:34: error: comparison is always true due to limited range of data type [-Werror=type-limits]
184 | g_return_val_if_fail (elt_size <= G_MAXSIZE / 2 - 1, NULL);
| ^~
glib.git/glib/gmacros.h:1090:25: note: in definition of macro ‘G_LIKELY’
1090 | #define G_LIKELY(expr) (expr)
| ^~~~
glib.git/glib/garray.c:184:3: note: in expansion of macro ‘g_return_val_if_fail’
184 | g_return_val_if_fail (elt_size <= G_MAXSIZE / 2 - 1, NULL);
| ^~~~~~~~~~~~~~~~~~~~
glib.git/glib/garray.c: In function ‘g_array_sized_new’:
glib.git/glib/garray.c:265:34: error: comparison is always true due to limited range of data type [-Werror=type-limits]
265 | g_return_val_if_fail (elt_size <= G_MAXSIZE, NULL);
| ^~
glib.git/glib/gmacros.h:1090:25: note: in definition of macro ‘G_LIKELY’
1090 | #define G_LIKELY(expr) (expr)
| ^~~~
glib.git/glib/garray.c:265:3: note: in expansion of macro ‘g_return_val_if_fail’
265 | g_return_val_if_fail (elt_size <= G_MAXSIZE, NULL);
| ^~~~~~~~~~~~~~~~~~~~
The (out caller-allocates) and (out callee-allocates) annotations are
meant for structured or pointer types. Plain old data types are just
regular out parameters and don't need the annotation about who
allocates them.
See: https://gitlab.gnome.org/GNOME/gjs/-/issues/386
GByteArray uses guint for storing the length of the byte array, but it
also has a constructor (g_byte_array_new_take) that takes length as a
gsize. gsize may be larger than guint (64 bits for gsize vs 32 bits
for guint). It is possible to call the function with a value greater
than G_MAXUINT, which will result in silent length truncation. This
may happen as a result of unreffing GBytes into GByteArray, so rather
be loud about it.
(Test case tweaked by Philip Withnall.)
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
Unify the creation of GPtrArray.
Maybe we will add yet another constructor for creating %NULL terminated
arrays. Unify the constructors by adding an internal helper method.
The alternative instead of adding a ptr_array_new() helper, would be to
let everybody call g_ptr_array_full(). For no strong reasons, choose
this approach because the compiler is more eager to inline the static
helper as it would inlining g_ptr_array_full().
g_ptr_array_extend_and_steal() leaves the GPtrArray in an invalid state,
so if you would try to append another pointer, it leads to a crash.
Also adjust the test case so that it would result in the crash (without
the fix).
Fixes: 0675703af08d ('Adding g_ptr_array_extend_and_steal() function to glib/garray.c')
Spotted by Mohammed Sadiq. `g_array_copy()` was doing a `memcpy()` of
the data from the old array to the new one, based on the reserved
elements in the old array (`array->alloc`). However, the new array was
allocated based on the *assigned* elements in the old array
(`array->len`).
So if the old array had fewer assigned elements than allocated elements,
`memcpy()` would fall off the end of the newly allocated data block.
This was particularly obvious when the old array had no assigned
elements, as the new array’s data pointer would be `NULL`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2049
