This reverts commit 851241f19a.
That commit avoids a performance regression but introduces a behavior regression:
changes to /etc/localtime have no effect for the remaining of the application's
runtime.
With the optimization introduced by the previous commit, we can pass NULL to
g_time_zone_new() repeatedly with no performance drawback, so we no longer have
to workaround this case.
Fixes: #2224
We cache GTimeZone instances to avoid expensive construction when the
same id is requested again.
However, if the NULL id is passed to g_time_zone_new(), we always
construct a new instance for the default/fallback timezone.
With the recent introduction of some heavy calculations[1], repeated
instance construction in such cases has visible performance impact in
nautilus list view and other such GtkTreeView consumers.
To avoid this, cache reference to a constructed default timezone and
use it the next time g_time_zone_new() is called with NULL argument,
as long as the default identifier doesn't change. We already did the
same for the local timezone[2].
Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/2204
Based on idea proposed by Sebastian Keller <skeller@gnome.org>.
[1] 25d950b61f
[2] 551e83662d
We have been passing a &resolved_identifier address around for multiple
functions to set it. Each function may either:
1. leaving it for the next function to set, if returning early;
2. set it to a duplicate of the passed identifier, if not NULL;
3. get a fallback value and set it, otherwise.
This can be simplified by setting it early to either:
1. a duplicate of the passed identifier, if not NULL;
2. a fallback value, otherwise.
This way we can avoid some unnecessary string duplication and freeing.
Also, on Windows, we avoid calling windows_default_tzname() twice.
But the main motivation for this change is enabling the performance
optimization in the next commit.
When the TZ environment variable is not set, we get the local timezone
identifier by reading specific files.
We are going to need these identifiers earlier, so split this logic into
its own function, in preparation for the next commit.
Based on idea proposed by Sebastian Keller <skeller@gnome.org>.
Rather than invalidating the cache by comparing `TZ` to the cached
timezone identifier, key entirely off the value of `TZ` (and a cached
copy of it).
This fixes the timezone cache being constantly invalidated if `TZ` is
`NULL` (which will always differ from the identifier of the default
local timezone which is constructed from `g_time_zone_new (NULL)`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2204
They shouldn’t be used to free a list from part-way through, as that
is confusing and will leave a dangling pointer from the previous list
element.
Spotted by Gary Kramlich in !1653.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Symbols on x86 are prefixed with an underscore
but symbols on x64/ARM/ARM64 are not.
Relevant information concerning the prefixes for the architectures
can be found in the vcpkg project [1,2], where arm and arm64 builds
are part of the CI.
Specifically, _WIN64 is defined on Windows ARM64, so this issue is
only visible when building on ARM32.
[1] 08e74979df/ports/glib/fix-arm-builds.patch
[2] https://github.com/microsoft/vcpkg/pull/6116
Some filesystems don't have meaningful access times under at least some
circumstances (see #2189, #2205). In this situation the traditional stat()
and related kernel interfaces have to put something meaningless in the
st_atime field, and have no way to signal that it is meaningless.
However, statx() does have a way to signal that the atime is meaningless:
if the filesystem doesn't provide a useful access time, it will unset
the STATX_ATIME bit (as well as filling in the same meaningless value
for the stx_atime field that stat() would have used, for compatibility).
We don't actually *need* the atime, so never include it in the required
mask. This was already done for one code path in commit 6fc143bb
"gio: Allow no atime from statx" to fix#2189, but other callers were
left unchanged in that commit, and receive the same change here.
It is not actually guaranteed that *any* of the flags in the
returned stx_mask will be set (the only guarantee is that items in
STATX_BASIC_STATS have at least a harmless compatibility value, even if
their corresponding flag is cleared), so it might be better to follow
this up by removing the concept of the required mask entirely. However,
as of Linux 5.8 it looks as though STATX_ATIME is the only flag in
STATX_BASIC_STATS that might be cleared in practice, so this simpler
change fixes the immediate regression.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2205
Signed-off-by: Simon McVittie <smcv@collabora.com>
glib/tests/date.c:778:17: error: comparison of integer expressions of
different signedness: ‘gint’ {aka ‘int’} and ‘long unsigned int’
778 | for (i = 0; i < G_N_ELEMENTS (check_years); i++)
| ^
glib/tests/collate.c:300:17: error: comparison of integer expressions
of different signedness: ‘gint’ {aka ‘int’} and ‘long unsigned int’
300 | for (i = 0; i < G_N_ELEMENTS (test); i++)
| ^
It is not allowed to be `NULL` or unset if requested by the file
attribute matcher. Derive it from the basename. This doesn’t handle the
situation of a failed UTF-16 to UTF-8 conversion very well, but will at
least return something.
Note that the `g_filename_display_basename()` function can’t be used as
`GWinHttpFile` provides its URI in UTF-16 rather than in the file system
encoding.
This fixes a crash when using GIMP on Windows. Thanks to lillolollo for
in-depth debugging assistance.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2194
This isn’t used in the build at all, it’s just a copy of the model file
which is uploaded in our Coverity configuration. This should be kept up
to date with changes in the file on the Coverity servers.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
`uri` is always non-`NULL` by the time the `fail` label is reached, so
drop the `NULL` pointer check. Inline the `fail` code since it’s only
used from two places.
Coverity CID: #1430970
Signed-off-by: Philip Withnall <withnall@endlessm.com>
For interoperability with libdbus, we want to use compatible timeouts.
In particular, this fixes a spurious failure of the `gdbus-server-auth`
test caused by libdbus and gdbus choosing to expire the key (cookie) at
different times, as diagnosed by Thiago Macieira. Previously, the libdbus
client would decline to use keys older than 7 minutes, but the GDBus
server would not generate a new key until the old key was 10 minutes old.
For completeness, also adjust the other arbitrary timeouts in the
DBUS_COOKIE_SHA1 mechanism to be the same as in libdbus. To make it
easier to align with libdbus, create internal macros with the same names
and values used in dbus-keyring.c.
* maximum time a key can be in the future due to clock skew between
systems sharing a home directory
- spec says "a reasonable time in the future"
- was 1 day
- now 5 minutes
- MAX_TIME_TRAVEL_SECONDS
* time to generate a new key if the newest is older
- spec says "If no recent keys remain, the server may generate a new
key", but that isn't practical, because in reality we need a grace
period during which an old key will not be used for new authentication
attempts but old authentication attempts can continue (in practice both
libdbus and GDBus implemented this logic)
- was 10 minutes
- now 5 minutes
- NEW_KEY_TIMEOUT_SECONDS
* time to discard old keys
- spec says "the timeout can be fairly short"
- was 15 minutes
- now 7 minutes
- EXPIRE_KEYS_TIMEOUT_SECONDS
* time allowed for a client using an old key to authenticate, before
that key gets deleted
- was at least 5 minutes
- now at least 2 minutes
- at least (EXPIRE_KEYS_TIMEOUT_SECONDS - NEW_KEY_TIMEOUT_SECONDS)
Based on a merge request by Philip Withnall.
Fixes: #2164
Thanks: Philip Withnall
Thanks: Thiago Macieira
Signed-off-by: Simon McVittie <smcv@collabora.com>
