- Commit 1bdcd05008 to fix RhBug:1045723
broke some funky java macros in Fedora which include line continuation
in the argument (comments 6-7 in the bug). That it ever worked seems
far more like luck than by design but since this seems to fix it...
- Test for terminating ')' existence before copying, otherwise we'll
end up walking over the edge of the world.
- Return address from doDefine() on error will likely differ after
this, whether that actually affects anything remains to be seen...
- Both doDefine() and doUndefine() assumed the macro string would
always fit into MACROBUFSIZ, which of course is true for any
normal use but we cant make such assumptions.
- In the case of %define/%global there are various other overrun-issues
that need further changes to fix.
- In the current macro implementation the check is simply far too
expensive to leave on always. Its useful though, so enable it
when tracing macro expansion, in which case you're probably
troubleshooting some macro-issues...
- Reword the message to something hopefully little more understandable,
change from error to a warning (only matters for output)
- Check unused macros at end of every scope, but unlike with parametrized
macros, dont actually delete
- Only whine once per unused macro due to the above to avoid spurious
output due to above
- This catches the common error in specs where %define is used in a scope
unintentionally, eg "%{!?foo: %define foo 1}" where the just defined
macro should actually fall out of scope upon the closing }, but
only gets erased if any parametrized macro is "called" and causes
insane behavior such as described in RhBug:552944. So at least we'll
warn on these situations.
- In practise doesn't change anything visible as the "macro not used"
message is disabled, but tracking a flag bit is saner than
strlen() + strchr() + bunch of other stuff which isn't even correct:
prior to this %## would've triggered "unused" errors which is plain
wrong, and complaining about unused %1 %2 ... isn't really right
either.
- Revert back to pre commit c22d5b1299
state wrt macro substitution. The commit does have fixes we want
but it also breaks valid cases which is not okay. We really need
a far more thorough macro test suite before these kind of changes,
and also need to do the changes in more controlled pieces.
- Commit c22d5b1299 changed the parsing
to require a valid name in %{name} macro, but this outlaws existing
uses such as %{foo_%{bar}_something}. Relaxing the %{name} form
to use as-is substitution as well allows these to work again.
- Makes the testcase from commit f082b5baa4
succeed. While the old behavior is non-sensical and most likely entirely
unintentional, we're changing a very long-standing behavior here (tested
back to rpm 4.4.x and almost certainly much much older than that) so
its entirely possible people are actually relying on the old
behavior. Lets see what breaks...
- Allow loading custom macro files from eg specs. This is quite different
from %include which inlines arbitrary content into a spec, but which
cannot be used for including macro files as their syntax is entirely
different. Both have their uses.
- rpm5.org also supports %load within macro files, we dont as I find the
imperative %load very alien in what's otherwise an entirely declarative
file "format"
- Using the already calculated macro arg + arglen for copying the
buffer instead of buggy local variant helps... This is as old as
the embedded lua interpreter in rpm, close to a decade.
- Besides fixing the segfault, this actually makes it behave like
other built-ins, evaluating to empty string when an empty arguments
or no arguments are passed.
- Previously various built-in macros without an actual argument,
eg %{basename:} would evaluate to "}" which makes no sense
whatsoever. With this, they evaluate to an empty string similarly
to when no argument is passed, eg %{basename}. Arguably these
should emit an error/warning instead but for now...
- This reverts commit 43a34e1554,
contrary to what the comment said the is NOT how other built-ins
behave, they evaluate to an empty string without an argument.
Better fix needed...
- %{lua:...} is used for invoking the embedded Lua interpreter, in
which case the script must be passed as the macro argument. If
no argument is passed, let it fall through to normal macro expansion.
This might not be the most sensible behavior possible but at least
its in line with what currently happens with other similar built-in
macros.
- Add the actual locking into macro context acquire + release. We're
using a mutex instead of rwlock because currently any macro can
involve defines and undefines and we dont know it beforehand. So
we just use a bigger hammer...
- The macro engine internals shouldn't need recursive mutexes but
unfortunately Lua macro bindings, which are limited to the
lock-on-entry interfaces can and do get called recursively
from macros so currently there's not much choice but to use
recursive mutex for the macro contexts. What makes it even uglier
is that there's no portable static initializer for recursive mutex
so we need to add yet another pthread-construct to safely dynamically
initialize the mutexes :(
- Of course this doesn't make bunch of threads simultaneously messing
with macros behave sane: if one thread tries to load new macros
and the other one frees them, they shouldn't crash but the
results are unlikely to be what the caller intended. The purpose
here is just to allow the occasional rpmExpand() and such to
complete without crashing and burning when multiple threads are
doing stuff like trying to read packages from disk.
- rpmLoadMacros() is a dumb name for what it does: it copies macros
from one context to another. The only actual use within rpm is
to copy back the cli macros to global context, but make the
internal helper more flexible by allowing copying to any context.
- rpmLoadMacros() is mostly just a dumb wrapper around copyMacros()
to grab locks and guard against copying global context to itself.
Adjust rpmInitMacros() to use copyMacros() as it already has a
lock on the global table, it just needs a lock on the cli contexts
as well.
- rpmInitMacros() already grabs the (theoretical) lock on entry
so we shouldn't try to grab it again, use the new lockless
loadMacroFile() version for the purpose.
- rpmLoadMacroFile() is now just a simple lock-wrapper around
loadMacroFile()
- rpmLoadMacroFile() already grabs the (theoretical) lock on entry
so we shouldn't try to grab it again, use the new lockless
defineMacro() version for the purpose.
- rpmDefineMacro() is now just a simple lock-wrapper around defineMacro()
- Add and use dummy macro context lock acquire + release functions,
ATM these only centralize the NULL -> rpmGlobalMacroContext translation.
- Eliminate the NULL context translation from internal interfaces,
ensure its always done at the outer levels already.
- At least rpmLoadMacros() and rpmLoadMacroFile() are fishy wrt
theoretical locking at this point and will require further internal
helpers...
- Rename addMacro() and delMacro() to pushMacro() and popMacro()
respectively, change all internal callers to use the push/pop
variants. Make addMacro() and delMacro() just dummy wrappers
for the internal variants.
- No functional changes, just paving way for adding thread-safety
mutex: we'll be locking at the external API boundary to keep
things simple, avoid nested locking etc.
- Avoid the expensive calloc() when creating a macro expansion buffer,
malloc() suffices just as well as long as we initialize the first
byte in the buffer. This is easily visible on wall-clock times.
- Avoid recalculating source string length: when caller doesn't
pass slen, we always calculate the string length for creating
a terminated copy, no need to redo it...
- Add a warning comment about the seemingly obvious optimization
breaking macro undefining itself, this is starting to look like
a recurring theme :)
- Macro name, opts and body must not be freed or otherwise written
to after initialization, make them const pointers to clarify
(and enforce) this. Arena is used to store whatever we need: it
always contains the macro body, sometimes also name and/or opts,
but these can be pointers to elsewhere too.
In the existing implementation, when a new macro is added, the whole
table has to be sorted again. Hence the cost of adding n macros is
worse than O(n^2), due to arithmetic progression.
This change drops all qsort(3) stuff altogether, by carefully preserving
table in sorted order. In findEntry routine, bsearch(3) is replaced
with customized binary search which tracks position for insertion.
In the addMacro routine, if a matching entry is not found, this
position is used for direct insertion, after the rest of the elements
are "shifted to the right" with memmove(3). Likewise, in delMacro
routine, the elements are shifted back to the left when the last macro
definition is popped. Technically, shifting half of the array with
memmove(3) is still O(n^2); however, modern CPUs process contiguous
memory in a very efficient manner, and glibc provides a fine-tuned
memmove(3) implementation.
Also, macro table entries are now allocated in a single chunk.
This change reduces rpm startup costs by factor of 6. Also, this change
improves specfile parser performance by a factor of 2 (e.g. the parse
time of texlive.spec is reduced from 67s to 35s).
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
This change introduces a separate routine to parse for valid macro
names. Valid macro names are either regular 3+ character identifiers,
or special names: "S", "P", "0", "#", "*", "**", macro options such as
"-o" and "-o*", and macro arguments such as "1". Other names are not
valid. This fixes a number of bugs seen earlier due to sloppy name
parsing: "%_libdir*" and "%01" were not expanded (these are now expanded
to e.g. "/usr/lib64*" and "<name>1", as expected). This also fixes
bugs in as-is substitution: "%!foo" was expanded to "%foo", and likewise
"%!!!" was expanded to "%" (and to "%<garbage>" at EOL).
Also, bad names in %name and %{name...} substitutions are now handled
differently. In %name form, the name is parsed tentatively; a silent
fall-back to as-is substitution is provisioned when no valid name can
be obtain. In %{name...} form, a failure to obtain a valid name is now
a syntax error. Furthermore, only 3 variants are syntactically valid:
%{name} proper, %{name:...}, and %{name ...}. This renders invalid
ambiguous macro substitutions such as the one found in FC18 lvm2.spec:
Requires: util-linux >= %{util-linux_version}
error: Invalid macro syntax: %{util-linux_version}
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
When the output from a command is empty, nothing stops doShellEscape from
chopping newlines past the beginning of the buffer. This problem was first
identified by Dmitry V. Levin in July 2009.
Also, there is an off-by-one error in replacing trailing '\n' with '\0'.
This problem, however, escaped the attention of Dmitry V. Levin in July 2009.
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
This will now issue a warning when macro definition is possibly
incorrect or ambigous, such as the one found in FC18 lvm2.spec:
%define util-linux_version 2.22.1
warning: Macro %util needs whitespace before body
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
'?' is returned by getopt when option is unknown, making hard to
pinpoint the actual bogus option...
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
Teach %prep and %uncompress how to handle 7zip tarballs, with
the mingw toolchain landing in fedora, this may be useful when
crossbuilding Windows sources compressed using 7zip (CxImage is
one such project).
- This isn't strictly needed as we're terminating the buffers "just in
case" all over the place but handling this centrally might allow
some day eliminating the other fluff...
- Oops, remember to reserve space for the trailing \0 when appending.
mb->nb holds the number of actual characters left in the buffer,
not the terminator. Fixes a regression introduced in rpm 4.9.x
dynamic macro realloction work (RhBug:431009 reprise)
- Actually protect against NULL mep (shouldn't happen but...)
- Remove bogus comment + add actually relevant comments
- Make the *mep reassignment more obvious by taking it out of
the if where its easily missed
- Replace dead NULL-assignments with a trash-n-burn memset()
- Fixup indentation to match general rpm style
- Lift the printbuffer accounting out of rpmlua into a struct of
its own (Funny thing, this looks a whole lot like the macro
expansion buffer and Good Ole StringBuf Brothers ... Boys ... Mam.
Unify them one of these days maybe)
- Replace the simplistic on/off printbuffer with a stack of buffers,
fixup the lone caller to use the new internal push/pop API.
- fopen() returns NULL on errors, never an opened stream with error
flag set. These are leftovers from past where rpmio fd was used
instead of FILE and probably the checks were bogus even back then too.
- A macro can undefine itself, and unless we grab a copy of it we'll
end up accessing already freed memory. Fixes a regression from
commit ebc4ceaaeb which assumed
a copy is not always needed.
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
- expandMacro() is big and ugly already, but handling the non-terminated
substrings there once and for all avoids having to ping-pong recurse
through couple of helpers to get there.
- Return the dynamically allocated expansion buffer to callers from
expandU(), except using a slightly less cryptic expandThis() name
for it now. Also deal with non-terminated strings centrally in
expandThis() instead of doing alloc + copy to terminate in every caller.
- Eliminate the underlying limitation of macro expansion limit by
growing the buffer as needed when adding characters to it. This
doesn't fix the entire issue yet however: expandU() and expandMacros()
are still limited to caller-specified buffer size, and output
from lua-macros isn't dynamically resized. One step at a time...
- Turn expandMacros() into a wrapper around internal doExpandMacros()
which returns the expanded string in a newly allocated buffer, use
the internal version for rpmExpand() too.
- In the cases where expandT() was called with strlen(source) we can
now just bypass it and call expandMacro() directly, avoiding an
unnecessary string copy.
- the "fix" breaks seemingly legitimate use in fedora font package
macros, possibly some funky interaction with %{lua: } macros or something
- this reverts commit f895acd285.
- Prior to this, local defines in constructs like %{!?foo: %define foo bar}
would remain defined until a parametrized macro gets called, causing
obscure and confusing errors in specs such as RhBug:551971 and countless
others. Use of %global is now always required for such constructs.
- define %_rpmconfigdir via %getconfdir in the main macro config, this
avoids it getting lost on macro reloads as happens when building
several packages at once
- one-shot to determine configuration base directory path from
RPM_CONFIGDIR environement or build-time default
- rpmfileutil is a bit strange place, this would really belong to librpm
but Lua initialization needs the path so...
- precalculate unexpanded size and allocate enough for that plus MACROBUFSIZ
for expansion
- typical allocation is way smaller than what gets allocated "just in case",
calculate expanded size and realloc to actual size to avoid wasting
memory
- expandMacro() wants the next \0 character to be returned, which might
or might not be the same as lastc passed to grabArgs()
- use memcpy() instead of memmove() for the copy, the areas can't overlap
- Use argvSplit() for splitting the macro path to components instead of
manual pointer-parsery.
- If URL's are to be supported or accepted at all (previous code attempted
to skip them), ':' is a very poor delimiter character.
- put some consistency into include ordering
- everything (apart from bits missed ;) is now ordered like this
1. "system.h"
2. other system includes
3. rpm public headers
4. rpm private headers
5. "debug.h"