Re: [Rpm-maint] FSM hooks for rpm plugin
Hi, I've started to wonder about the hook names though: open and close seem out of place especially here (and to lesser degree elsewhere). Perhaps these hooks should simply follow the pre/post pattern even in the names, ie FILE_PRE and FILE_POST. I think it'd be more descriptive wrt what they actually do, and leave open and close free for possible later use for the cases where a kind of open+close (files from payload stream) is actually occurring. Sure, this is an easy change and I just did it. But the one below has made me a headache :( This gets quite ugly indeed, better refactor the surrounding code (which is ugly in itself) to allow for nice clean pairing. Oh and btw, this version of the patch introduces a new asymmetry case: if the open-hook fails, close-hook wont get called. It'll need something like open-hook just flagging postpone without terminating the loop, and adjusting the rest of the code to deal with it. I'll have a look at this, how hard can it be ;) I have written and rewritten now this part and still it looks ugly or even worse :( Flagging postpone is actually easy as well as not breaking out of loop in the beginning, but the early return is a small nightmare. If you introduce a variable tracking this, smth like hardbreak and setting it at that point, then you have a number of stupid if() causes, which make the whole constructions completely unreadable in addition to handling postpone and rc status (I have to walk thought this with pen and big white sheet to track the cases). Other option is goto xyz to bypass this but I am not sure this is any better, but might be easier to read. What do you think is better? I have also fixed the other two issues you pointed in the other email. Best Regards, Elena. smime.p7s Description: S/MIME cryptographic signature ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] FSM hooks for rpm plugin
On 02/04/2013 02:18 PM, Reshetova, Elena wrote: Hi, I've started to wonder about the hook names though: open and close seem out of place especially here (and to lesser degree elsewhere). Perhaps these hooks should simply follow the pre/post pattern even in the names, ie FILE_PRE and FILE_POST. I think it'd be more descriptive wrt what they actually do, and leave open and close free for possible later use for the cases where a kind of open+close (files from payload stream) is actually occurring. Sure, this is an easy change and I just did it. But the one below has made me a headache :( This gets quite ugly indeed, better refactor the surrounding code (which is ugly in itself) to allow for nice clean pairing. Oh and btw, this version of the patch introduces a new asymmetry case: if the open-hook fails, close-hook wont get called. It'll need something like open-hook just flagging postpone without terminating the loop, and adjusting the rest of the code to deal with it. I'll have a look at this, how hard can it be ;) I have written and rewritten now this part and still it looks ugly or even worse :( Flagging postpone is actually easy as well as not breaking out of loop in the beginning, but the early return is a small nightmare. If you introduce a variable tracking this, smth like hardbreak and setting it at that point, then you have a number of stupid if() causes, which make the whole constructions completely unreadable in addition to handling postpone and rc status (I have to walk thought this with pen and big white sheet to track the cases). Other option is goto xyz to bypass this but I am not sure this is any better, but might be easier to read. What do you think is better? I meant that I'll have a look at streamlining this particular jungle first in a way that (hopefully) makes adding the hooks easy or at least easier. There are all sorts of suspicious spots in there... I have also fixed the other two issues you pointed in the other email. Ok, good... but the patch seems to be missing from your mail :) - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] missing rpm-4.11 API doc on rpm.org
Hi The rpm.org web site hasn't been fully updated for 4.11 yet. http://www.rpm.org/wiki/Docs#DeveloperDocumentation lacks rpm-4.11 API doc... See you ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [PATCH 5/5] Fix horrible bug in freeing macro arguments
After a macro which takes options/arguments is called, such as %systemd_post,
freeArgs() frees ALL macros *previously defined in specfile*. For example,
in amanda.spec, after scriptlet sections which invoke %systemd_* macros are
processed, %amanda_user and %amanda_group in %files sections become undefined,
resulting in malformed owner/group file metadata.
$ rpm -qp --qf '[%{FILEUSERNAME}:%{FILEGROUPNAME}\n]'
amanda-3.3.2-2.fc18.x86_64.rpm |
sort | uniq -c | sort -n | tail
3 %amanda_user:%amanda_group
104 root:root
This change installs additional name check in freeArgs(), so that, upon
finishing macro call, only macro arguments are freed.
---
rpmio/macro.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/rpmio/macro.c b/rpmio/macro.c
index eee7716..d124b87 100644
--- a/rpmio/macro.c
+++ b/rpmio/macro.c
@@ -625,6 +625,9 @@ freeArgs(MacroBuf mb)
assert(me);
if (me-level mb-depth)
continue;
+ int c = *me-name;
+ if (!(c == '*' || c == '#' || c == '-' || risdigit(c)))
+ continue;
if (strlen(me-name) == 1 strchr(#*0, *me-name)) {
if (*me-name == '*' me-used 0)
skiptest = 1; /* XXX skip test for %# %* %0 */
--
1.8.1
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [PATCH 4/5] Optimize readLine routine
This change introduces the following optimizations:
- Line chunking used to be done with per-character loop; it now uses
strchr(line, '\n') call.
- Likewise, the balancing check has been enhanced with strpbrk(3).
Callgrind annotations for 'rpmspec -P glibc.spec', previous commit:
94,796,525 PROGRAM TOTALS
33,242,383 rpmio/argv.c:argvCount
9,886,925 build/parseSpec.c:readLine
3,964,158 glibc-2.16-75f0d304/malloc/malloc.c:_int_malloc
...
Callgrind annotations for 'rpmspec -P glibc.spec', this commit:
88,013,018 PROGRAM TOTALS
33,242,383 rpmio/argv.c:argvCount
3,964,158 glibc-2.16-75f0d304/malloc/malloc.c:_int_malloc
...
---
build/parseSpec.c | 66 ---
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/build/parseSpec.c b/build/parseSpec.c
index 9429454..6cd6bc0 100644
--- a/build/parseSpec.c
+++ b/build/parseSpec.c
@@ -290,23 +290,30 @@ static int copyNextLineFromOFI(rpmSpec spec, OFI_t *ofi)
{
/* Expand next line from file into line buffer */
if (!(spec-nextline *spec-nextline)) {
- int pc = 0, bc = 0, nc = 0;
const char *from = ofi-readPtr;
- char ch = ' ';
- while (from *from ch != '\n') {
- ch = spec-lbuf[spec-lbufOff] = *from;
- spec-lbufOff++; from++;
-
- if (spec-lbufOff = spec-lbufSize) {
- spec-lbufSize += BUFSIZ;
- spec-lbuf = realloc(spec-lbuf, spec-lbufSize);
- }
+ const char *end = strchr(from, '\n');
+ size_t len;
+ if (end == NULL) {
+ len = strlen(from);
+ end = from + len;
+ }
+ else {
+ end++; /* grab newline */
+ len = end - from;
+ }
+ if (len + 1 spec-lbufSize - spec-lbufOff) {
+ spec-lbufSize += BUFSIZ + len + 1;
+ spec-lbuf = xrealloc(spec-lbuf, spec-lbufSize);
}
+ memcpy(spec-lbuf + spec-lbufOff, from, len);
+ spec-lbufOff += len;
spec-lbuf[spec-lbufOff] = '\0';
- ofi-readPtr = from;
+ ofi-readPtr = end;
/* Check if we need another line before expanding the buffer. */
- for (const char *p = spec-lbuf; *p; p++) {
+ int pc = 0, bc = 0, nc = 0;
+ for (const char *p = spec-lbuf;
+ (p = strpbrk(p, \\\n%{}())) != NULL; p++)
switch (*p) {
case '\\':
switch (*(p+1)) {
@@ -328,7 +335,6 @@ static int copyNextLineFromOFI(rpmSpec spec, OFI_t *ofi)
case '(': if (pc 0) pc++; break;
case ')': if (pc 0) pc--; break;
}
- }
/* If it doesn't, ask for one more line. */
if (pc || bc || nc ) {
@@ -359,22 +365,23 @@ static int copyNextLineFromOFI(rpmSpec spec, OFI_t *ofi)
return 0;
}
+/* Logical chunking into lines after macro expansion */
static void copyNextLineFinish(rpmSpec spec, int strip)
{
-char *last;
-char ch;
-
-/* Find next line in expanded line buffer */
-spec-line = last = spec-nextline;
-ch = ' ';
-while (*spec-nextline ch != '\n') {
- ch = *spec-nextline++;
- if (!risspace(ch))
- last = spec-nextline;
+/* This line is to be processed */
+spec-line = spec-nextline;
+
+/* Next line is after newline character */
+char *end = strchr(spec-line, '\n');
+if (end == NULL) {
+ /* Last line */
+ size_t len = strlen(spec-line);
+ end = spec-line + len;
+ spec-nextline = end;
}
-
-/* Save 1st char of next line in order to terminate current line. */
-if (*spec-nextline != '\0') {
+else {
+ spec-nextline = end + 1;
+ /* Save 1st char of next line in order to terminate current line */
spec-nextpeekc = *spec-nextline;
*spec-nextline = '\0';
}
@@ -382,8 +389,11 @@ static void copyNextLineFinish(rpmSpec spec, int strip)
if (strip STRIP_COMMENTS)
handleComments(spec-line);
-if (strip STRIP_TRAILINGSPACE)
- *last = '\0';
+if (strip STRIP_TRAILINGSPACE) {
+ while (end spec-line risspace(end[-1]))
+ end--;
+ *end = '\0';
+}
}
static int readLineFromOFI(rpmSpec spec, OFI_t *ofi)
--
1.8.1
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
