The VM harness (tests/vm/) builds and exercises the tools on real Debian and
Alpine guests, driving the serial console (no sshd/cloud-init dependency).
Results on Debian 12 (glibc 2.36, gcc 12.2): 12/12 checks pass, 0 warnings
with -Wall -Wextra, including a real 128M -> 256M resize and the rollback
save/restore/reject paths.
Results on Alpine 3.20 (musl 1.2.5): the build cannot start. configure stops
at "reiserfsprogs requires a method to add printf modifiers" because
reiserfscore/prints.c depends on glibc's register_printf_specifier(3) and
<printf.h>, which musl does not provide. This is pre-existing and out of
scope here; measured at 341 use sites across 20+ files, all routed through
reiserfs_warning()/reiserfs_info(). Documented in tests/vm/RESULTS.md.
Fix the one real defect the harness surfaced:
fsck/semantic_check.c: check_check_regular_file() is called with NULL for
new_ih (line 466), so the block that would fill *new_ih is unreachable and
check_semantic_pass() never returns RELOCATED through that path. The
"case RELOCATED:" arm still reads relocated_ih.ih_key, which gcc 12.2
reports as "may be used uninitialized" (-Wmaybe-uninitialized); gcc 16 does
not, because it proves the path dead. Zero-initialise the struct and say why.
Verified: 0 warnings on gcc 12.2 after the change, and the Debian battery
stays at 12/12.
Build: add --enable-warnings (-Wall -Wextra); the tree is now warning-free
with it, so distributions can opt in without risking a surprise -Werror
breakage from a future compiler.
Real bugs fixed (each verified by a reproducer):
- lib/io.c: open_rollback_file() read 28 bytes into char[28] with no NUL
and then strcmp()'d it against a 28-character magic, reading one byte
past the array. The condition was also inverted, so a *valid* rollback
file was rejected -- but only sometimes: the missing NUL made the result
depend on stack garbage (verified: -O0 rejects, -O2 accepts). The whole
--save-rollback/--rollback-changes path was unreliable. Now the buffer is
sized for the terminator, zeroed, and the comparison is `!= 0`.
Verified end to end: save -> reopen -> restore reports "2 blocks
restored, 0 skipped"; a random file is rejected.
- reiserfscore/reiserfslib.c, reiserfscore/journal.c: asprintf() was
called with &fs->fs_file_name / &fs->fs_j_file_name, but those fields
are `char *`, not `char **`. Passing a `char **` where `char *` is
expected wrote through the wrong pointer -- silent memory corruption on
any allocation. Fixed by using the new checked helper.
- reiserfscore/node_formats.c: `code < 0` on an unsigned parameter was
dead code; the guard never fired.
- resize_reiserfs: missing `break` after -j made -j fall into -f, so
"-j dev" silently also forced the operation. Sign-compare on the
count_blocks() ternary and on count + blocks.
- lib/misc.c: strncat with bound == strlen(src) (equivalent to strcat) into
char[256] from a char[256] dirent name -- up to 261 bytes. Replaced with
snprintf plus an explicit fit check.
- lib/misc.c: die() used vsprintf into a fixed 1024-byte buffer.
- strncpy with bound == destination size (mkreiserfs, tune, progbar) never
terminated; replaced with snprintf.
Allocator: drop the homegrown magic-header allocator. It wrote "_mem_begin_"
/ "mem_end" through strcpy inside the allocation, kept the size in an
`int`, and its only safety check (checkmem) was never called -- every call
site is commented out. Replaced with a size_t header that is over-aligned
via max_align_t, so malloc alignment guarantees still hold. expandmem()
now takes the old size from the header instead of a caller-supplied value
that could disagree with it.
Warnings: annotate the deliberate fallthroughs (fsck pass cascade, unit
suffix parsing, percentage parsing), fix the remaining sign-compare and
missing-field-initializer sites, and compute addresses of packed members
via char* + offsetof instead of &member.
Portability: asm/types.h -> linux/types.h, malloc.h and sys/vfs.h dropped
from includes.h (nothing used them), sys/vfs.h -> sys/statfs.h for
statfs(2).
Version: reiserfsprogs.spec said 3.6.23 while configure.ac said 3.6.25.
Remove the dead `#if 0` main() from hashes.c.
Add .github/workflows/build.yml: gcc, clang, --enable-warnings and
--enable-io-failure-emulation, each followed by a format/check/inspect
smoke test on a real image.
Verified: ./autogen.sh && ./configure --enable-warnings && make is clean
from scratch (0 warnings, 51 compile/link steps). mkreiserfs, reiserfsck
--check, debugreiserfs -S, reiserfstune -l and a real resize_reiserfs
128M -> 256M all succeed, and fsck reports the resized image clean.
There are many cases where functions take pointers to keys, item heads,
paths, etc and don't modify them. Let's annotate them properly.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
By annotating structure members and other on-disk structures as little
endian, we can catch bugs more easily.
The code was already endian safe except for three minor spots:
1) The badblock list code was initializing its key in an endian-unsafe way.
2) Indirect block pointer sequence printing was treating cpu-order values
as little endian.
3) reiserfsck was trying to do endian switching on the lost+found key,
but it was before it was even initialized to anything, so we can
just remove that completely.
These issues were fixed as part of this commit.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
This patch does a quick search and replace:
B_N_PITEM_HEAD() -> item_head()
B_N_PDELIM_KEY() -> internal_key()
B_N_PKEY() -> leaf_key()
B_N_PITEM() -> item_body()
And the item_head version:
B_I_PITEM() -> ih_item_body()
I_ENTRY_COUNT() -> ih_entry_count()
And the treepath variants:
get_ih() -> tp_item_head()
PATH_PITEM_HEAD() -> tp_item_head()
get_item() -> tp_item_body()
... which makes the code much easier on the eyes.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Over the years, the reiserfs kernel code has been gradually cleaned up
to be more readable. This patch does the same to the progs code.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
In order to convert reiserfscore into a externally-usable library,
we need to rename structures so they won't collide with other
namespaces.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
lgcosta
Jeff Mahoney