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
this will be a massive change compared to the usual stuff. however the
gains will be worth it:
* we gain lots of additional animated image support.
* and we'll gain _even_ more format support as imlib2 adds them, without needing
any change in our code-base.
* about ~300 LoC will be purged once we remove our internal gif and webp loader.
as for when to remove the internal loaders, a good time might be when debian
upgrades their imlib2, currently it seems to be at v1.7.5, which doesn't support
animated images.
as of now, nsxiv will continue to build with the internal gif/webp loaders
(assuming they were enabled in config.mk) if imlib2 version is below 1.8.0 and
will print out a deprecation notice.
and if imlib2 version supports multi-frame then it will simply ignore the
internal loaders and use the imlib2 one.
in other words, users shouldn't need to do anything on their side. everything
that previously functioned will continue to function regardless of the user's
imlib2 version (though they might see the annoying deprecation notice if the
imlib2 version doesn't support multi-frame images).
known issue:
* image loading performance can be noticeably worse in
imlib2 versions below 1.9.0
Closes: https://codeberg.org/nsxiv/nsxiv/issues/301
Closes: https://codeberg.org/nsxiv/nsxiv/issues/300
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/373
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
* Imlib2 supports modifying gamma, brightness and contrast directly
while sxiv only supports gamma. Makes sense to extend it to brightness
and contrast as well.
* Since color corrections need to be aware of each other, they have been
refactored into one centralized function.
* This also makes the code more hackable as it makes it easier to add
more color correction functions without them interfering with each
other.
Co-authored-by: 0ion9 <finticemo@gmail.com>
Co-authored-by: NRK <nrk@disroot.org>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/396
Reviewed-by: NRK <nrk@disroot.org>
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
Co-authored-by: Berke Kocaoğlu <kberke@metu.edu.tr>
Co-committed-by: Berke Kocaoğlu <kberke@metu.edu.tr>
allows for developers to more easily run the analysis locally before
opening a pull request if they wish.
also disables a noisy warning (bugprone-assignment-in-if-condition)
producing too many false positives.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/395
Reviewed-by: Berke Kocaoğlu <kberke@metu.edu.tr>
Co-authored-by: NRK <nrk@disroot.org>
Co-committed-by: NRK <nrk@disroot.org>
this basically just extracts the logic that was previously inside
`main()` into a seperate function `add_entry()` so that it can be used
for accepting entries form stdin as well.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/382
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/383
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
the length member needed to be zero-ed before we started decoding.
this was causing unintended behavior of playing an animated webp longer
than it should under slideshow mode.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/381
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
the way `imlib_load_image()` works, is that it only does a lightweight
signature/metadata check. it doesn't actually decode the image. which
means that a file that has valid metadata but invalid content would get
loaded successfully.
`imlib_image_get_data_for_reading_only()` basically forces imlib to
decode the data, and thus reveal any malformed images so we can reject
it (see commit f0266187).
however, this is a spurious way of achieving the goal at hand. imlib2
already offers an `_immediately` variant which decodes the data
immediately. so just use that instead of spuriously using the "get_data"
function to force a decode.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/380
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
* ensure static variables comes after non-static ones
* remove depreciated DATA32 type
* prefer `sizeof(expression)` over `sizeof(Type)`.
* silence a -Wsign warning
* {gif,webp} loader: use a pointer to reduce code-noise
* gif loader: allocate in one place
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/374
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
this will hopefully catch issues like [337] in the future.
not using gcc and/or clang since we have 5 build options right now,
which means 2^5 = 32 different combination. using gcc/clang would take
too much resources and time; meanwhile tcc is lightning fast.
[337]: https://codeberg.org/nsxiv/nsxiv/pulls/337
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/338
Reviewed-by: eylles <eylles@noreply.codeberg.org>
currently the error messages have the shortopt hardcoded in them, even
when the user actually entered a longopt:
$ ./nsxiv --framerate v
nsxiv: Invalid argument for option -A: v
and as far as I see, there's no way to *reliably* tell weather we're
processing a longopt or a shortopt. perhaps we can do some shenanigangs
with `optind` but that seems finicky at best.
and it seems like other coreutils which support longopt has similar issues:
$ xargs --max-procs=z
xargs: invalid number "z" for -P option
utils like `grep` and `head` seems to work-around it by not mentioning the flag:
$ head --lines=z
head: invalid number of lines: ‘z’
$ grep --max-count=l
grep: invalid max count
this patch does the same thing as `grep/head` and omits the flag from
the error message.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/368
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/371
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
strtol() returns a `long`, but we're storing the result in an `int`
which might end up getting truncated. change `n` to `long` and guard
against >INT_MAX arguments in cases where it matters.
use a float for storing argument of `-S`
change `opt.slideshow` to `unsigned` similar to `img.ss.delay`
Co-authored-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/367
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
simply running nsxiv with `--anti-alias` will enable anti-aliasing, and
running it with `--anti-alias=no` will disable it.
the cli flag will overwrite the config.h default.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/349
handle a rare, but possible case of argc being 0, in which case argv[0]
would be null.
note that both POSIX and ISO C standard allow argc to be 0 and in
practice this can be triggered via calling `exec(3)` family of functions
with NULL as the first `argv`.
* disable `cert-err33-c` and `readability-identifier-length` check
which causes warnings on clang-tidy v14.
* disable all "readability" checks by default, instead just opt-into the
useful ones (such as duplicate include).
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/365
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
* link to online man-page
* rename: "N-R-K" -> "NRK"
* separate multiple links with a comma.
otherwise the links get messed up due to `[] []` being valid syntax for
reference style links with the 2nd `[]` serving as `id`.
* prefer codeberg links over github ones
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/362
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
* run_key_handler: make the logic easier to follow
* remove timeout_t
the typedef is not needed. inline the declaration similar to the other
static structs.
* simplify estrdup
reuse emalloc, instead of calling malloc and null-checking.
* win_clear: initialize `e` right away
* process_bindings: explicitly check against NULL
most pointer checks in the codebase do explicit check.
* use a named constant instead of magic number
also changes the padding from 3 to 4 bytes according to [0]. but i
couldn't find any situtation where this mattered, so perhaps the current
padding is enough. but doesn't hurt adding one more byte.
[0]: https://nullprogram.com/blog/2017/10/06/
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/356
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
Uses [optparse] to add support for long-opts. optparse is posix
compliant with getopt(3) and thus would be backwards compatible.
It does not have any dependency (not even the c standard library!) and
is C89 compatible and thus fits our current code-style.
[optparse]: https://github.com/skeeto/optparse
Note that we're using a couple `pragma`-s to silence some harmless
warnings. This should be portable because these pragma-s don't change the
behavior of the program. Furthermore, C standard mandates that unknown
pragma's should be ignored by the compiler and thus would not result in
build failure on compilers which do not recognize them.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/328
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/332
Reviewed-by: eylles <eylles@noreply.codeberg.org>
_SC_PHYS_PAGES isn't POSIX and might not be defined. in such case, just
return back `CACHE_SIZE_FALLBACK`.
NOTE: POSIX says the `names` in `sysconf()` are "symbolic constants" not
necessarily macros. So we might end up returning the fallback in some
cases where `_SC_PHYS_PAGES` *was* available, but not defined as a
macro. which is not ideal, but nothing fatal.
in practice, this shouldn't be an issue since most systems seems to
define them to be macros, i've checked Glibc, Musl, OpenBSD, FreeBSD and
Haiku.
also add a (useful) comment on `config.h` describing the effect higher
cache size has.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/354
This is mainly just to reduce the amount of files in the project root.
The criteria of what gets into `etc/` are the following:
* The file should not be necessary for building nsxiv. This excludes the
`icon/*` stuff since that's needed by `window.c`.
* The file shouldn't have any valid reason to stay in the project root.
This excludes things like `README.md`, `.gitignore` etc.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/350
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
Fixes a link that previously 404'd
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/351
Reviewed-by: NRK <nrk@disroot.org>
Co-authored-by: Nan <nan42@waifu.club>
Co-committed-by: Nan <nan42@waifu.club>
the current code is quite hacky and complex as it mixes multiple pointers. all
of this complexity is unnecessary. drop it by introducing an explicit scratch
buffer instead of implicitly abusing `arl->filename` as one. this also reduces
some unnecessary allocation overhead.
additionally, the argument to arl_setup must be the result of `realpath(3)` (as
commented in `nsxiv.h`). instead of commenting it, assert it.
and lastly, rename `arl_setup` to `arl_add` since it's not doing any "setup"
but rather *adding* a file to watch.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/342
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
* install unversioned `llvm` so that it pulls the latest one.
* disable `uninitvar` check on cppcheck as it catches some false positives and
gcc/clang/clang-tidy are good at catching uninitialized variables already.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/348
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
excluding the `.png` icons.
this also fixes the wrong `.patch` and `.diff` extensions checks.
no clue why we were trying to find `.patch` and `.diff` to begin with.
i think i probably copy pasted the command i tested on nsxiv-extra repo
or somewhere else without changing the extensions to `.c` and `.h`.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/345
this was one of the cases which got missed in 591be8c, if someone starts
with `-t` the statusbar will remain at "Loading ...". Once we're done
loading all the thumbnail, make sure to open_info() so that `thumb-info`
gets called.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/341
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
mixing signed and unsigned types in comparison can end up having
unintended results. for example:
if (-1 < 1U)
printf("true\n");
else
printf("false\n");
previously we silenced these warnings, instead just fix them properly
via necessary casting, and in cases where the value cannot be negative
(e.g width/height members) make them unsigned.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/336
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
currently, in case of an empty xresources we would fail to init:
nsxiv: Error allocating color ''
instead, just reject empty value and use the fallback.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/339
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/340
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
Co-authored-by: NRK <nrk@disroot.org>
Co-committed-by: NRK <nrk@disroot.org>
utf8_decode() may return an errors, in which case the returned codepoint
might be garbage. the issue can be tested by adding the following to
`image-info` which produces invalid utf8 sequences:
base64 -d << EOF
9JCAgPSQgIH0kICC9JCAg/SQgIT0kICF9JCAhvSQgIf0kICI9JCAifSQgIr0kICL9JCAjPSQgI30
kICO9JCAj/SQgJD0kICR9JCAkvSQgJP0kICU9JCAlfSQgJb0kICX9JCAmPSQgJn0kICa9JCAm/SQ
gJz0kICd9JCAnvSQgJ8=
EOF
on my system, this leads to the statusbar being filled with empty boxes.
check for returned error and skip the iteration.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/327
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
the following combination currently fails to build:
$ make HAVE_LIBFONTS=0 HAVE_LIBWEBP=1
this is probably because one of the font library was including <stdio.h>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/337
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
Co-authored-by: NRK <nrk@disroot.org>
Co-committed-by: NRK <nrk@disroot.org>
* includes are sorted alphabetically
* their grouping and layout is the following:
- nsxiv.h will be the first include
- followed by any internal headers (e.g "commands.h" "config.h")
- followed by system headers (<stdlib.h> etc)
- followed by third party headers (X.h libwebp etc)
* also add `llvm-include-order` check to clang-tidy so that it can catch
unsorted includes during CI.
* rm unused include <sys/types.h>
* move <sys/time.h> to main.c, it's the only file that needs it.
* move TV_* macros to main.c
* let *.c files explicitly include what they need instead of including
them at nsxiv.h
Originally we stopped doing links in the CHANGELOG since it harmed
readability when reading the raw markdown file. However we can use
reference styles links instead.
* Pros
- Doesn't harm readability.
- We can link to proper thread on the proper repo (`CodeBerg/nsxiv`
for newer threads, `nsxiv-record` for wiped ones)
- The links will be clickable on the [webpage]
- Both GitHub and Codeberg support reference style links, as well as
lowdown (which we use for building the webpage)
[webpage]: https://nsxiv.codeberg.page/changelog/
* Cons
- Slightly higher maintainance burden.
- Larger file size.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/325
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
cppcheck warns about passing a plain NULL to construct_argv()
main.c:254:36: portability: Passing NULL after the last typed argument to a variadic function leads to undefined behaviour. [varFuncNullUB]
fidx, fcnt, w, h, z, NULL);
in our case, POSIX mandates `NULL` to be defined as `(void *)0` rather
than a plain `0` [0]. and void pointers are required to have the same
representation and size as char pointers [1]. so this is not an issue we
need to worry about.
[0]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stddef.h.html#tag_13_47
[1]: https://port70.net/~nsz/c/c99/n1256.html#6.2.5p27
the warnings on r_readdir(), img_load_gif() and strcpy seems to be false
positives. the warning about fmt being unused is valid, but not worth
fixing with additional #ifdef guards.
use `assert` to silence the false positive cases when possible,
otherwise use a NOLINT comment with an explanation.
it's possible for the close() calls to override the errno resulting in
incorrect error message being printed.
call error() immediately to avoid such possibilities.
also refactor a couple conditions to avoid doing multiple checks.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/321
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
NRK
Berke Kocaoğlu
explosion-mental
Nan
zood
Berke Kocaoğlu