The macro contexts being static structs, they get their destructor
automatically called at program end. Which counter-intuitively *causes*
a leak if macros are stored in an STL container and rpmFreeMacros()
isn't called, because then the container clears itself and the macros
are left dangling, whereas without the destructor they are still
reachable. freeMacros() obviously wants to be a member function but
trying to keep things in the struct land for now.
Doesn't really change anything as of now, but this is needed for moving
the macro storage to C++ container.
C++ strings can hold \0's so we can use it for convenient storage of
keyids, passing them to C requires special attention anyhow. And then
we can easily replace all array bookkeeping and lookup fubar with
STL map. We could just as easily use unordered_map, but map matches
what it did before so...
freeArgs() only popped any local macros once, so if a local macro was
pushed multiple times, whether through %define or multiple identical
options getting passed, we leaked any remaining macros to the outside
scope.
Simply pop the local macros in a loop to fix. Have the internal
popMacro() return the previous pointer (if any) to simplify the job.
We even had an expected-fail test for this, now passing, but add a
more straightforward reproducer too.
This bug was circa 26 years old. Some might call it vintage at this point.
Fixes: #3056
A stupid newbie C++ error, array allocation would've been "new uint8_t[nb]"
and requiring delete[] as well. Use a vector instead, then we don't
need to manually free or track the size.
We really need to test the optional io types too, eww.
Rename some internal structs to common style, the dust is thick in this
part of the cellar. In particular "lzfile" as a struct name clashes with
our local variables, don't do that.
Some background + rationale that will apply to a lot more commits going
forward and not going to keep repeating it all, so using this as a cover
letter of sorts:
Converting rpm to something resembling C++ is going to be a multi-year
operation, and what is being done here is just the first step of many.
Moving to native C++ allocation seems like an important first step as it
allows using non-trivial C++ data types (such as strings) in said constructs
(with malloc/free, destructors do not run). Also, it brings down the
number of raw C allocations with their ugly casts. It's worth nothing
that we'll never be free of raw mallocs entirely as some of the allocations
we return are expected to be free()'d from C, such as the output
parameter of rpmDigestFinal(). So we just want to get to the state where
ALL C-style allocations are for data going across the API border as
quickly as possible, the mixed state of things is ugly and fragile.
In many places rpm is relying on calloc() or memset() to zero-initialize
structs, but this cannot be used on C++ structs with "non-trivial" data
types such as strings, STL containers and the like. The {}
initialization is the nearest C++ counterpart for that. It'd be safer
to place the initialization(s) in the struct members directly but we're
shooting for minimal changes at this point, that'll come later.
Finally, we are using "naked new" because many of our pointers are going
across the API border to C where smart pointers simply cannot be used,
not without extra tricks anyhow. We don't want to worry about that just
now. We'll be using "naked new" for internal pointers too initially, again
just to keep changes minimal.
rpmlogCallbackData is already a pointer type, we don't want a pointer
to a pointer for this. Kinda surprising it actually worked, but then
it's just a void pointer so...
Upstream Lua headers lack C++ guards, argh. Lump them all into rpmlua.h
and deal with it centrally there. In the past we've avoided including
Lua there but now that Lua is mandatory it doesn't cause other issues.
C lets us do this but in C++ needs to run destructors at end of scope,
so they better be properly initialized...
These are all over the place but so trivial putting them to different
commits doesn't make much sense.
Forward declarations to structs like we have in rpmio isn't legitimate
C++, as a minimal bandaid solution declare them as extern in the
internal header, and limit visibility.
This really belongs to a separate function in the first place, and doing
so allows us to turn 'b' into a const char *, which in turn makes
the assignment to string literal "" in the url2path case legitimate.
Fun times and no functional changes.
Previously rpmGlob() has behaved as if GLOB_NOMAGIC was passed, ie
return non-globbed patterns as is without bothering to check if
there are actual matches. This makes for weird and surprising behavior:
~/.notthere would return no match, but /home/user/.notthere returns a
match. Which makes no sense whatsoever.
Rather than clone all the glob() options to the rpm interface, link
this to the NOCHECK flag: the callers who are prepared to deal with
non-existent files are already using RPMGLOB_NOCHECK, and all the
rest actually expect all the matches to be there.
Now that we have an alternative to building without Rust, it's time
to say bye to this old thing. We will not support the parser but
preserve minimal hooks in cmake to allow building with it, at least
for a transition period:
https://github.com/rpm-software-management/rpmpgp_legacyFixes: #2414
Push the build option into the rpmpgp_legacy directory so it doesn't
show up at all unless the directory is present, and rename it to
WITH_LEGACY_OPENPGP to better reflect the status: it's hardly internal
if it lives in a separate repo, and it's something you should not use
going forward.
Add a terse README to the rpmpgp_legacy directory as the initial to-be
repo description to explain the status and intentionally vague build
instructions: if you don't know then you really should not.
Also add a separate COPYING file there: the parser originates from
rpmio/ so it falls under rpm's dual license, just simplify the text
that makes no sense in the new context.
For bootstrapping purposes, having rpm depend on Rust is painful, but
directing people to unmaintained crypto code as an alternative is
hair-raising. As a middle ground, let rpm be built without OpenPGP
support at all, which at least gives you a functional rpm and rpm-build
even if you can't sign or verify signatures.
Achieving this is a moderately complex dance which can't meaningfully
be split into multiple commits because everything is interconnected:
Add a new WITH_SEQUOIA option to control use of Sequoia, on by default.
When Sequoia is disabled, default to a newly added dummy PGP implementation
instead which just returns error on everything. And finally, if the
older WITH_INTERNAL_OPENPGP is enabled, use the old PGP implementation.
As the intent is to cut out rpmpgp_legacy to a separate repository,
sanity requires that we also split the openssl/libgcrypt code at the
digest/signature fault line. It's not ideal, but the alternative of
having unused crypto code on which an external component depends on
is just not sustainable. This way, the signature side of things is
quite neatly cut off with the PGP stuff. The diff looks big but there
are no code/functional changes in the libgcrypt/openssl split.
Rpm scriptlets should have no business dealing with this level of
detail in process control, rpm.execut() is much saner and safer
for scriptlet needs. We can't just remove because this is a public
API with known users (eg glibc in Fedora), but we can at least document
these as deprecated with noisy warnings.
fork() and exec() are the main "problems" here, but wait() and
redirect2null() become meaningless once you take the first two away.
Fixes: #2420
This should be an error for consistency with other macro argument
checking but as there appear to be usages in the wild, make it a
warning for now.
Fixes: #2932
This changes the behaviour of %dirname to something sane. No longer
return the argument unchanged if there is no / found. Also handle
several cornercases properly.
Resolves: #2928
We checked for libintl in the top-level CMakeLists.txt but then never
used it for anything. This only ever worked on glibc where this all
is bundled in. Unfortunately Intl only becomes an importable target
in cmake >= 3.20 which is too new for us to rely on for now.
Python bindings are omitted here because we don't have any translated
messages in there. Whether we should is another topic.
This regressed when we axed our internal glob copy in commit
66fa46c006. Luckily GLOB_ONLYDIR is only
an optimization so we can just skip it if not available.
Panu Matilainen