by writing to a tmpfile first, and then renaming it to the desired
target - multiple nsxiv instances writing thumbnail at the same time are
guaranteed not to stomp over one another.
rename() is guaranteed to be atomic by POSIX. however, it can fail with
EXDEV if both the files don't reside in the same filesystem. and so we
cannot make the tmpfile something like "/tmp/nsxiv-XXXXXX".
instead, create the tmpfile inside the cache_dir to reduce chances of
EXDEV occuring.
currently the logic of when to open/close script is scattered around the
entire code-base which is both ugly and error-prone.
this patch centralizes script handling by remembering the relevant
information on each redraw and then comparing it with the previous
information to figure out whether something changed or not.
this also fixes a bug where scripts weren't being called in thumbnail
mode when mouse was used for selecting a different image.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/475
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/477
Reviewed-by: eylles <eylles@noreply.codeberg.org>
4b67816 only partially fixed this issue. because imlib2's cache may not
have sub-second granularity, there still exists a time-frame while the
`mtime` has not yet been updated but we might be trying to reload the
image due to receiving an inotify event in which case imlib2 will end up
giving us the old frame.
imlib2 v1.12.0 adds a function that allows us to decache any frames
associated with a filename. this allows us to invalidate the cache
manually instead of relying on `mtime`.
but if that's not available due to older imlib2, then forcefully reload
the raw frames and decache them. this has the unfortunate cost that if
`mtime` *was* updated properly then we'll end up loading that image
twice.
fixes: https://codeberg.org/nsxiv/nsxiv/issues/456
don't capitalize "berg" in "Codeberg" spelling. Codeberg's official
sites does not seem to do so.
switch from "GPLv2" to "GPL-2.0-or-later" according to the spdx short
identifier: https://spdx.org/licenses/GPL-2.0-or-later.html
explicitly mention that Imlib2 needs to be built with X11 support.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/470
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
currently the autoreload feature of nsxiv is a bit unreliable because we
try to load at the very first event we received. however, the writer
might not be done writing and so we might try to load a truncated image
(and fail).
in the following ascii diagram, function S represents sleep and `+` sign
represents writes by the writer. because we set the sleep (of 10ms) at
the first event, subsequent writes by the writer doesn't influence our
reload logic:
S(10) load()
nsxiv | |
writer + + + + (done)
time(ms): 00 05 10 15
after this patch, (assuming function T (re)sets a timeout), we will keep
(re)setting a timeout on new events giving the writer more time to
finish:
T(10) T(10) T(10) T(10) load()
nsxiv | | | | |
writer + + + + (done)
time(ms): 00 05 10 15 20 25
while this patch makes things significantly more robust, the problem
here is inherently unsolvable since there's no way to tell whether the
writer is done writing or not.
for example, if user does something like `curl 'some.png' > test.png`
then curl might stop for a second or two in the middle of writing due to
internet issues - which will make nsxiv drop the image.
this patch also increases the autoreload delay from 10ms to now 128ms
instead to decrease chances of false failures.
ref: 6ae2df6ed5
partially-fixes: https://codeberg.org/nsxiv/nsxiv/issues/456
commit-message-by: NRK
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/459
Reviewed-by: eylles <eylles@noreply.codeberg.org>
1. `c99` doesn't exist on openbsd
2. `c99` behaves weirdly on macos (doesn't support -Wall)
since make will already set CC (required by POSIX if the implementation
supports C, see `man 1p make`) to something appropriate (or it might be
set in the environment) go ahead and remove explicitly setting it on our
end.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/455
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
these document improvements and/or removal of unnecessary code for when
we will require a higher minimum version of Imlib2.
all the comments have been prefixed with "UPGRADE: " for easy grepping.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/457
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
a lot of application which allow selecting features at build time seem
to output the build config with `--version` or similar (e.g ffmpeg).
aside from giving the user information about the feature set the binary
was compiled with (in case the user didn't compile it themselves, e.g on
a binary distro) it can also (possibly) help when submitting bug
reports.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/462
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
might as well make things consistent instead of having certain values
that can be NULL and certain that cannot.
some of the default values are still kept in config.h for example
reasons.
* switch to git ls-files to avoid picking up any other local .c files
* enable assertions during static analysis since we used some assertions to
disable/silence certain warnings.
* update TCC commit hash to a more recent one
* parallelize static analysis
cppcheck already has -j argument to parallelize it's analysis and
provide results faster, clang-tidy unfortunately doesn't.
so use xargs -P to archive parallel execution. on my system this brings
down the analysis time from ~27s to ~5s.
the fedora copr repo is no longer being updated since the maintainer of
it, mamg22, no longer uses nsxiv in his daily setup (and thus stopped
contributing to nsxiv as well).
he has requested the repo and his email to be removed from the project.
so go ahead and honor that request.
also take this as an opportunity to remove some long inactive
maintainers from the CURRENT MAINTAINERS section of the manpage.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/448
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
slight addendum to 657080a7e5
instead of disabling asserts by adding -DNDEBUG to config.mk, this
disables asserts by default in the source code itself. this way, if
someone compiles with `make CFLAGS="-O3 -march=native"` without knowing
about asserts/-DNDEBUG then he won't accidentally get a build with
assertions in it.
this basically makes the assertions opt-in, if someone wants it, he'll
need to *explicitly* set `-DDEBUG` to get it. so that it's not possible
to accidentally end up with assertions enabled.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/447
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
gif spec [0] doesn't mention what to do when "Delay Time" is 0.
apng spec [1] states:
| If the the value of the numerator is 0 the decoder should render the
| next frame as quickly as possible, though viewers may impose a
| reasonable lower bound.
webp spec [2]:
| the interpretation of frame duration of 0 (and often <= 10) is
| implementation defined.
so it seems that it's safe to set a default delay for 0 delay frames,
which is what the older gif and webp loaders were already doing. do the
same for the imlib2 multi-frame loader as well.
[0]: https://www.w3.org/Graphics/GIF/spec-gif89a.txt
[1]: https://wiki.mozilla.org/APNG_Specification
[2]: https://developers.google.com/speed/webp/docs/riff_container#animation
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/445
Reviewed-by: eylles <eylles@noreply.codeberg.org>
this avoids overwriting the left side bar,
which might contain more important information,
for e.g output of the thumb-info script.
Co-authored-by: A1337Xyz <blindwizard@tutanota.com>
Co-authored-by: NRK <nrk@disroot.org>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/446
Reviewed-by: eylles <eylles@noreply.codeberg.org>
Reviewed-by: NRK <nrk@disroot.org>
Co-authored-by: a1337xyz <a1337xyz@noreply.codeberg.org>
Co-committed-by: a1337xyz <a1337xyz@noreply.codeberg.org>
the multiframe loaders sets the color modifier to NULL but doesn't
restore it before returning. this results in a imlib2 developer warning
if you try to change brightness/contrast on a multiframe image (which
doesn't have alpha).
ensure that the color modifier is restored before returning under all
paths.
Reported-by: Madhu
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/440
Reviewed-by: eylles <eylles@noreply.codeberg.org>
by default imlib2 doesn't check the file's timestamp to avoid disk
activity when loading from cache. however, this ends up breaking our
autoreload functionality on multi-frame images.
the reason why single frame images weren't broken was because
`img_load()` calls `imlib_image_set_changes_on_disk()`, which tells
imlib2 to check the timestamp before loading from cache.
do the same thing for the multi-frame loader as well.
additionally add a comment to img_load() explaining what's going on.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/436
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/437
Reviewed-by: eylles <eylles@noreply.codeberg.org>
clang-tidy currently flags the following:
util.c:57:8: error: 'ptr' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage,-warnings-as-errors]
ptr = realloc(ptr, size);
the analysis here is correct, but if realloc fails, we simply exit so
there's no real "leak".
moreover this check is not very useful for nsxiv's codebase because we
do not use naked realloc(), instead we use the erealloc wrapper that
exits on failure. so just disable the warning entirely instead of
changing the source code to silence the false positive.
assertions are for debugging purposes, and so shouldn't be enabled for
"release" builds. disable it by default by using `-DNDEBUG`.
`-O2` on gcc/clang will result it slightly better binary. on tcc it'll
be ignored. and since -O is specified by POSIX there shouldn't be any
portability concern.
additionally add a (commented out) recommended debug build for gcc/clang
with address and undefined sanitizers turned on.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/424
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/435
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
Reviewed-by: eylles <eylles@noreply.codeberg.org>
The last time[0] this was discussed, no-one was against it but no-one
was confident in it either and so it was added to nsxiv-extra as a
patch.
But now that enough time has passed, it seems like there's a pretty high
demand for something like this because there's plenty of use-cases that
use nsxiv as a "picker" where it's meant to quickly pick a single file.
And so add this as a convenient default key-bind.
[0]: https://codeberg.org/nsxiv/nsxiv-record/pulls/42
Co-authored-by: eylles <ed.ylles1997@gmail.com>
Co-authored-by: NRK <nrk@disroot.org>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/432
Reviewed-by: NRK <nrk@disroot.org>
Co-authored-by: eylles <eylles@noreply.codeberg.org>
Co-committed-by: eylles <eylles@noreply.codeberg.org>
otherwise, it ends up applying to the manpage and git commit messages
too.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/433
Reviewed-by: eylles <eylles@noreply.codeberg.org>
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
bf6c062 tried to fixed the thumbnail leak, but it was done inside a
`if (n+1 < filecnt)` branch, meaning the thumbnail was still leaking
away whenever the last file was removed.
we need to unload the thumb regardless of whether it's in the middle or
not. this bug was caught due to the recent `assert`s that were added in
01f3cf2.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/422
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/423
Reviewed-by: eylles <eylles@noreply.codeberg.org>
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
removes a couple maintainers who have never been active in the
development.
there are a couple other maintainers have not been active in a while,
but was somewhat active in the past. so keep their names in it
(for now at least).
also re-arrange the entires a bit based on activity.
this section was added almost 10 years ago (see commit 60f84190f) back
when sxiv was pretty new and didn't have many contributors.
the situation has obviously changed now, especially with `nsxiv` we have
a fair amount of contributors now. so it makes no sense to special case
a couple of them.
instead of silently ignoring bogus arguments (i.e programming errors),
which can make debugging harder, it's better to assert them so that they
get caught faster in debug builds.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/406
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
now that we have long-opts, we don't have to worry about exhausting the
alphabet list for short-opts. so adding a cli flag to set/unset the
checker background makes sense.
ref: https://codeberg.org/nsxiv/nsxiv/issues/404
reported by thread-sanitizer.
the sighandler's spoiled `errno` was causing xlib to incorrectly assume some
error occurred and thus causing the crash described in #391.
to reproduce:
* Open an nsxiv window
* Open another terminal and run the following:
var=$(pidof nsxiv); while :; do kill -s SIGCHLD $var; done
putting the `pid` into a variable is actually important because doing
`$(pidof nsxiv)` inside the loop makes it really hard to reproduce the
issue, I presume because of the extra process invocation it was sending
less SIGCHLD and so putting it into a variable avoids that overhead and
is able to generate more signals.
instead of reaping the zombies manually, we now pass the
`SA_NOCLDSTOP|SA_NOCLDWAIT` for SIGCHLD instead so that the zombies are
reaped automatically.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/391
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/411
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
looks like some update to the markdown parser at codeberg broke our readme.
just use a simple `*` instead (escaped by `\` to be safe)
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/402
posix_spawn() is designed especially for this purpose, and thus it's
much more lightweight and efficient than manually fork/dup/exec-ing.
on my system, it improves the performance of spawn() by about 10x. given
that we make frequent calls to potentially multiple scripts, the
increased efficiency will add up overtime.
using posix_spawn() also simplifies the logic quite a bit, despite the
very verbose function names. however it does make cleanup a bit more
complicated.
this patch uses the linux kernel style cleanup strategy [0] (which I'm
personally not a huge fan of, but it fits this situation quite nicely)
with a "stack-like" unwinding via `goto`-s.
additionally simplify the spawn() API by taking in {read,write}fd
pointers and returning the pid instead of using some custom struct.
this coincidently also fixes#299
[0]: https://www.kernel.org/doc/html/v4.10/process/coding-style.html?highlight=goto#centralized-exiting-of-functions
NRK
David Gowers
padv
explosion-mental
a1337xyz
blk_750
eylles
Berke Kocaoğlu