[Rpm-maint] [PATCH] Fix rpmlog printing of off_t in fdConsume.

2017-10-05 Thread Mark Wielaard
off_t is a somewhat weird type because the only thing that is defined
about it is that it is signed. Depending on whether you are compiling
on a 32 or 64 system and/or whether compiling for large file support
it has different sizes. Currently compiling build/pack.c on 32bit arches
will give a warning because the size doesn't match the %ld format
specifier. Change it to %jd and explicitly cast it to intmax_t to make
it print consistently on any arch.

Signed-off-by: Mark Wielaard 
---
 build/pack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/build/pack.c b/build/pack.c
index 46816cf..1348b5f 100644
--- a/build/pack.c
+++ b/build/pack.c
@@ -431,8 +431,8 @@ static rpmRC fdConsume(FD_t fd, off_t start, off_t nbytes)
 };
 
 if (left) {
-   rpmlog(RPMLOG_ERR, _("Failed to read %ld bytes in file %s: %s\n"),
-   nbytes, Fdescr(fd), Fstrerror(fd));
+   rpmlog(RPMLOG_ERR, _("Failed to read %jd bytes in file %s: %s\n"),
+  (intmax_t) nbytes, Fdescr(fd), Fstrerror(fd));
 }
 
 return (left == 0) ? RPMRC_OK : RPMRC_FAIL;
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [PATCH] Fix rpmlog warning in lzopen_internal from too big length specifier.

2017-10-05 Thread Mark Wielaard
memlimit is defined as uint32_t, but was printed as %lu.
Change to %u to avoid a gcc warning.

Signed-off-by: Mark Wielaard 
---
 rpmio/rpmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
index d8b8840..c7cbc32 100644
--- a/rpmio/rpmio.c
+++ b/rpmio/rpmio.c
@@ -839,7 +839,7 @@ static LZFILE *lzopen_internal(const char *mode, int fd, 
int xz)
 
if (threads != (int)mt_options.threads)
rpmlog(RPMLOG_NOTICE,
-   "XZ: Adjusted the number of threads from %d to 
%d to not exceed the memory usage limit of %lu bytes",
+   "XZ: Adjusted the number of threads from %d to 
%d to not exceed the memory usage limit of %u bytes",
threads, mt_options.threads, memlimit);
}
 #endif
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [PATCH] Avoid warning about mbAppendStr if lui support isn't enabled.

2017-10-05 Thread Mark Wielaard
Guard the whole mbAppendStr function with ifdef WITH_LUA to avoid a
warning when lua support isn't enabled because it is only used inside
the doLua function (which is empty unles WITH_LUA is defined).

Signed-off-by: Mark Wielaard 
---
 rpmio/macro.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rpmio/macro.c b/rpmio/macro.c
index 0f90aaf..b6d8900 100644
--- a/rpmio/macro.c
+++ b/rpmio/macro.c
@@ -394,6 +394,7 @@ static void mbAppend(MacroBuf mb, char c)
 mb->nb--;
 }
 
+#ifdef WITH_LUA
 static void mbAppendStr(MacroBuf mb, const char *str)
 {
 size_t len = strlen(str);
@@ -405,6 +406,8 @@ static void mbAppendStr(MacroBuf mb, const char *str)
 mb->tpos += len;
 mb->nb -= len;
 }
+#endif
+
 /**
  * Expand output of shell command into target buffer.
  * @param mb   macro expansion state
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Mark Wielaard
On Thu, 2017-10-05 at 15:36 +0200, Thierry Vignaud wrote:
> On 5 October 2017 at 14:07, Igor Gnatenko
>  wrote:
> > > For the record
> > > BTW, the latest issue I've with rpm-4.14 is that its testsuite
> > > has
> > > several debugsources related failures  when building it with rpm-
> > > 4.14
> > > tpoo
> > > http://pkgsubmit.mageia.org/uploads/done/cauldron/core/release/20
> > > 1710
> > > 05114000.tv.duvel.20120/rpm-4.14.0-
> > > 0.rc2.4.mga7/build.0.20171005114003.log
> > > Have you tried building rpm-4.14 with rpm-4.14?
> > 
> > I think the problem is that you have enabled producing debugsource
> > in
> > RPM while we have it in redhat-rpm-config in Fedora. I've seen same
> > failures when I tried to enable debugsource packages directly in
> > RPM...
> 
> Nope.
> It's not enabled in rpm but it is in the pkg equivalent to
> redhat-rpm-config (rpm-mageia-setup), which is installed by default
> in
> build chroots
> After all we do want rpm-debugsource
> Also di

This sentence doesn't seem finished.

In Fedora rawhide rpm is build with rpm-4.14-rc in the build roots.
It shows zero fails. All 383 tests behaved as expected.
https://kojipkgs.fedoraproject.org//packages/rpm/4.14.0/0.rc2.5.fc28/data/logs/x86_64/build.log
It also generates separate debugsource and debuginfo subpackages for rpm itself.
https://koji.fedoraproject.org/koji/buildinfo?buildID=979215

The tests should enable/disable various debug generation settings
explicitly.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Thierry Vignaud
On 5 October 2017 at 13:27, Thierry Vignaud  wrote:
> Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
> earlier because this already shows it's not a regression in 4.14.x
> but
> something else. A bug in perl-RPM4 perhaps, as compiling it with -Og
> makes
> the crash go away, other optimization levels make it blow up with
> different
> levels of spectacular. I dont see anything obvious in there but that
> doesn't
> mean much, I know diddly about perl and its extensions.




 I ran it with some added debugging on rpm side which I'm more
 familiar
 with, and the crash happens because a totally garbage pointer is
 passed
 to
 headerFree(). Well indeed, it was passing the address of the header
 pointer
 variable as the header itself into the callback, and when you try do
 stuff
 with that, well...

 This fixes it:

 diff --git a/src/RPM4.xs b/src/RPM4.xs
 index 04c65ee..6604477 100644
 --- a/src/RPM4.xs
 +++ b/src/RPM4.xs
 @@ -246,7 +246,7 @@ static void *
 s_what = "INST_START";
 if (h) {
 mXPUSHs(newSVpv("header", 0));
 -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
 ));
 +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
 h));
 #ifdef HDRPMMEM
>>>
>>>
>>>
>>>
>>>
>>> Oh and you'll want to fix the debug printf too, even though it's
>>> obviously
>>> harmless (but then useless for debugging):
>>>
 PRINTF_NEW(bless_header, , -1);
>>>
>>>
>>>
>>> ^^
>>
>>
>>
>>
>> Blech, one of those days...
>>
>> The above is closer to mark but still not quite right and will crash
>> too,
>> only in a different way because the refcount is wrong. Here's the real
>> deal:
>>
>> diff --git a/src/RPM4.xs b/src/RPM4.xs
>> index 04c65ee..f7cfd33 100644
>> --- a/src/RPM4.xs
>> +++ b/src/RPM4.xs
>> @@ -246,9 +246,9 @@ static void *
>>s_what = "INST_START";
>>if (h) {
>>mXPUSHs(newSVpv("header", 0));
>> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
>> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>> headerLink(h)));
>>#ifdef HDRPMMEM
>> -PRINTF_NEW(bless_header, , -1);
>> +PRINTF_NEW(bless_header, h, -1);
>>#endif
>>}
>>break;
>>
>>   - Panu -
>>
>
> Thanks
> Now remains the issue with several builds on the same spec object
>

 Yeah, I was just about to write about that.

 I totally misunderstood the case initially (that's what happens if you
 try
 to understand the world by peeking through a keyhole), I thought your
 testcase was looking at the number of files by itself and finding one
 more
 than expected. When the error actually comes from rpm itself.
>>>
>>>
>>> Yeah we had a misunderstanding there :-)
>>>
 That file entries get messed when reusing the same spec is probably an
 ages
 old issue, it's just that until now rpm didn't sanity check the file
 data.

 The right thing to do is to reload the spec, I doubt it ever *really*
 worked
 correctly.
>>>
>>>
>>> OK I'll commit my fix in the VC then.
>>> There's still one last testsuite failure with RPM4 when verifying a
>>> rpmdb after rebuilding it :
>>>
>>> $ LC_ALL=C perl -Iblib/lib -Iblib/arch t/05transaction.t 2>&1|grep -A2
>>> -B36 "not ok"
>>> error: rpmdb: Enhancename: No such file or directory
>>> error: db5 error(2) from db->verify: No such file or directory
>>> (...)
>>> error: rpmdb: Name: No such file or directory
>>> error: db5 error(2) from db->verify: No such file or directory
>>> not ok 4 - Verify empty
>>> #   Failed test 'Verify empty'
>>> #   at t/05transaction.t line 24.
>>>
>>> The initdb creates the proper files, but after rebuilddb, there's only
>>> "Packages" (+ the __db.000 env)
>>> This used to work.
>>> Any idea?
>>>
>>
>> Yeah, see
>> https://github.com/rpm-software-management/rpm/commit/b62d85d78b07d6d43ca26fd7cda489c636a5796b
>> :)
>>
>> (and yes that'll be in 4.14.0 too)
>>
>> - Panu -
>
> Thanks.

For the record
BTW, the latest issue I've with rpm-4.14 is that its testsuite has
several debugsources related failures  when building it with rpm-4.14
tpoo
http://pkgsubmit.mageia.org/uploads/done/cauldron/core/release/20171005114000.tv.duvel.20120/rpm-4.14.0-0.rc2.4.mga7/build.0.20171005114003.log
Have 

Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Thierry Vignaud
On 5 October 2017 at 12:03, Panu Matilainen  wrote:
 Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
 earlier because this already shows it's not a regression in 4.14.x
 but
 something else. A bug in perl-RPM4 perhaps, as compiling it with -Og
 makes
 the crash go away, other optimization levels make it blow up with
 different
 levels of spectacular. I dont see anything obvious in there but that
 doesn't
 mean much, I know diddly about perl and its extensions.
>>>
>>>
>>>
>>>
>>> I ran it with some added debugging on rpm side which I'm more
>>> familiar
>>> with, and the crash happens because a totally garbage pointer is
>>> passed
>>> to
>>> headerFree(). Well indeed, it was passing the address of the header
>>> pointer
>>> variable as the header itself into the callback, and when you try do
>>> stuff
>>> with that, well...
>>>
>>> This fixes it:
>>>
>>> diff --git a/src/RPM4.xs b/src/RPM4.xs
>>> index 04c65ee..6604477 100644
>>> --- a/src/RPM4.xs
>>> +++ b/src/RPM4.xs
>>> @@ -246,7 +246,7 @@ static void *
>>> s_what = "INST_START";
>>> if (h) {
>>> mXPUSHs(newSVpv("header", 0));
>>> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>>> ));
>>> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>>> h));
>>> #ifdef HDRPMMEM
>>
>>
>>
>>
>>
>> Oh and you'll want to fix the debug printf too, even though it's
>> obviously
>> harmless (but then useless for debugging):
>>
>>> PRINTF_NEW(bless_header, , -1);
>>
>>
>>
>> ^^
>
>
>
>
> Blech, one of those days...
>
> The above is closer to mark but still not quite right and will crash
> too,
> only in a different way because the refcount is wrong. Here's the real
> deal:
>
> diff --git a/src/RPM4.xs b/src/RPM4.xs
> index 04c65ee..f7cfd33 100644
> --- a/src/RPM4.xs
> +++ b/src/RPM4.xs
> @@ -246,9 +246,9 @@ static void *
>s_what = "INST_START";
>if (h) {
>mXPUSHs(newSVpv("header", 0));
> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
> headerLink(h)));
>#ifdef HDRPMMEM
> -PRINTF_NEW(bless_header, , -1);
> +PRINTF_NEW(bless_header, h, -1);
>#endif
>}
>break;
>
>   - Panu -
>

 Thanks
 Now remains the issue with several builds on the same spec object

>>>
>>> Yeah, I was just about to write about that.
>>>
>>> I totally misunderstood the case initially (that's what happens if you
>>> try
>>> to understand the world by peeking through a keyhole), I thought your
>>> testcase was looking at the number of files by itself and finding one
>>> more
>>> than expected. When the error actually comes from rpm itself.
>>
>>
>> Yeah we had a misunderstanding there :-)
>>
>>> That file entries get messed when reusing the same spec is probably an
>>> ages
>>> old issue, it's just that until now rpm didn't sanity check the file
>>> data.
>>>
>>> The right thing to do is to reload the spec, I doubt it ever *really*
>>> worked
>>> correctly.
>>
>>
>> OK I'll commit my fix in the VC then.
>> There's still one last testsuite failure with RPM4 when verifying a
>> rpmdb after rebuilding it :
>>
>> $ LC_ALL=C perl -Iblib/lib -Iblib/arch t/05transaction.t 2>&1|grep -A2
>> -B36 "not ok"
>> error: rpmdb: Enhancename: No such file or directory
>> error: db5 error(2) from db->verify: No such file or directory
>> (...)
>> error: rpmdb: Name: No such file or directory
>> error: db5 error(2) from db->verify: No such file or directory
>> not ok 4 - Verify empty
>> #   Failed test 'Verify empty'
>> #   at t/05transaction.t line 24.
>>
>> The initdb creates the proper files, but after rebuilddb, there's only
>> "Packages" (+ the __db.000 env)
>> This used to work.
>> Any idea?
>>
>
> Yeah, see
> https://github.com/rpm-software-management/rpm/commit/b62d85d78b07d6d43ca26fd7cda489c636a5796b
> :)
>
> (and yes that'll be in 4.14.0 too)
>
> - Panu -

Thanks.
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Panu Matilainen

On 10/05/2017 01:00 PM, Thierry Vignaud wrote:

On 5 October 2017 at 11:40, Panu Matilainen  wrote:

Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
earlier because this already shows it's not a regression in 4.14.x but
something else. A bug in perl-RPM4 perhaps, as compiling it with -Og
makes
the crash go away, other optimization levels make it blow up with
different
levels of spectacular. I dont see anything obvious in there but that
doesn't
mean much, I know diddly about perl and its extensions.




I ran it with some added debugging on rpm side which I'm more familiar
with, and the crash happens because a totally garbage pointer is passed
to
headerFree(). Well indeed, it was passing the address of the header
pointer
variable as the header itself into the callback, and when you try do
stuff
with that, well...

This fixes it:

diff --git a/src/RPM4.xs b/src/RPM4.xs
index 04c65ee..6604477 100644
--- a/src/RPM4.xs
+++ b/src/RPM4.xs
@@ -246,7 +246,7 @@ static void *
s_what = "INST_START";
if (h) {
mXPUSHs(newSVpv("header", 0));
-mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
+mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, h));
#ifdef HDRPMMEM





Oh and you'll want to fix the debug printf too, even though it's
obviously
harmless (but then useless for debugging):


PRINTF_NEW(bless_header, , -1);



^^




Blech, one of those days...

The above is closer to mark but still not quite right and will crash too,
only in a different way because the refcount is wrong. Here's the real
deal:

diff --git a/src/RPM4.xs b/src/RPM4.xs
index 04c65ee..f7cfd33 100644
--- a/src/RPM4.xs
+++ b/src/RPM4.xs
@@ -246,9 +246,9 @@ static void *
   s_what = "INST_START";
   if (h) {
   mXPUSHs(newSVpv("header", 0));
-mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
+mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
headerLink(h)));
   #ifdef HDRPMMEM
-PRINTF_NEW(bless_header, , -1);
+PRINTF_NEW(bless_header, h, -1);
   #endif
   }
   break;

  - Panu -



Thanks
Now remains the issue with several builds on the same spec object



Yeah, I was just about to write about that.

I totally misunderstood the case initially (that's what happens if you try
to understand the world by peeking through a keyhole), I thought your
testcase was looking at the number of files by itself and finding one more
than expected. When the error actually comes from rpm itself.


Yeah we had a misunderstanding there :-)


That file entries get messed when reusing the same spec is probably an ages
old issue, it's just that until now rpm didn't sanity check the file data.

The right thing to do is to reload the spec, I doubt it ever *really* worked
correctly.


OK I'll commit my fix in the VC then.
There's still one last testsuite failure with RPM4 when verifying a
rpmdb after rebuilding it :

$ LC_ALL=C perl -Iblib/lib -Iblib/arch t/05transaction.t 2>&1|grep -A2
-B36 "not ok"
error: rpmdb: Enhancename: No such file or directory
error: db5 error(2) from db->verify: No such file or directory
(...)
error: rpmdb: Name: No such file or directory
error: db5 error(2) from db->verify: No such file or directory
not ok 4 - Verify empty
#   Failed test 'Verify empty'
#   at t/05transaction.t line 24.

The initdb creates the proper files, but after rebuilddb, there's only
"Packages" (+ the __db.000 env)
This used to work.
Any idea?



Yeah, see 
https://github.com/rpm-software-management/rpm/commit/b62d85d78b07d6d43ca26fd7cda489c636a5796b 
:)


(and yes that'll be in 4.14.0 too)

- Panu -
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Thierry Vignaud
On 5 October 2017 at 11:40, Panu Matilainen  wrote:
>> Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
>> earlier because this already shows it's not a regression in 4.14.x but
>> something else. A bug in perl-RPM4 perhaps, as compiling it with -Og
>> makes
>> the crash go away, other optimization levels make it blow up with
>> different
>> levels of spectacular. I dont see anything obvious in there but that
>> doesn't
>> mean much, I know diddly about perl and its extensions.
>
>
>
> I ran it with some added debugging on rpm side which I'm more familiar
> with, and the crash happens because a totally garbage pointer is passed
> to
> headerFree(). Well indeed, it was passing the address of the header
> pointer
> variable as the header itself into the callback, and when you try do
> stuff
> with that, well...
>
> This fixes it:
>
> diff --git a/src/RPM4.xs b/src/RPM4.xs
> index 04c65ee..6604477 100644
> --- a/src/RPM4.xs
> +++ b/src/RPM4.xs
> @@ -246,7 +246,7 @@ static void *
>s_what = "INST_START";
>if (h) {
>mXPUSHs(newSVpv("header", 0));
> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, h));
>#ifdef HDRPMMEM




 Oh and you'll want to fix the debug printf too, even though it's
 obviously
 harmless (but then useless for debugging):

>PRINTF_NEW(bless_header, , -1);


^^
>>>
>>>
>>>
>>> Blech, one of those days...
>>>
>>> The above is closer to mark but still not quite right and will crash too,
>>> only in a different way because the refcount is wrong. Here's the real
>>> deal:
>>>
>>> diff --git a/src/RPM4.xs b/src/RPM4.xs
>>> index 04c65ee..f7cfd33 100644
>>> --- a/src/RPM4.xs
>>> +++ b/src/RPM4.xs
>>> @@ -246,9 +246,9 @@ static void *
>>>   s_what = "INST_START";
>>>   if (h) {
>>>   mXPUSHs(newSVpv("header", 0));
>>> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
>>> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>>> headerLink(h)));
>>>   #ifdef HDRPMMEM
>>> -PRINTF_NEW(bless_header, , -1);
>>> +PRINTF_NEW(bless_header, h, -1);
>>>   #endif
>>>   }
>>>   break;
>>>
>>>  - Panu -
>>>
>>
>> Thanks
>> Now remains the issue with several builds on the same spec object
>>
>
> Yeah, I was just about to write about that.
>
> I totally misunderstood the case initially (that's what happens if you try
> to understand the world by peeking through a keyhole), I thought your
> testcase was looking at the number of files by itself and finding one more
> than expected. When the error actually comes from rpm itself.

Yeah we had a misunderstanding there :-)

> That file entries get messed when reusing the same spec is probably an ages
> old issue, it's just that until now rpm didn't sanity check the file data.
>
> The right thing to do is to reload the spec, I doubt it ever *really* worked
> correctly.

OK I'll commit my fix in the VC then.
There's still one last testsuite failure with RPM4 when verifying a
rpmdb after rebuilding it :

$ LC_ALL=C perl -Iblib/lib -Iblib/arch t/05transaction.t 2>&1|grep -A2
-B36 "not ok"
error: rpmdb: Enhancename: No such file or directory
error: db5 error(2) from db->verify: No such file or directory
(...)
error: rpmdb: Name: No such file or directory
error: db5 error(2) from db->verify: No such file or directory
not ok 4 - Verify empty
#   Failed test 'Verify empty'
#   at t/05transaction.t line 24.

The initdb creates the proper files, but after rebuilddb, there's only
"Packages" (+ the __db.000 env)
This used to work.
Any idea?
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Panu Matilainen

On 10/05/2017 12:34 PM, Thierry Vignaud wrote:

On 5 October 2017 at 10:15, Panu Matilainen  wrote:

Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
earlier because this already shows it's not a regression in 4.14.x but
something else. A bug in perl-RPM4 perhaps, as compiling it with -Og makes
the crash go away, other optimization levels make it blow up with different
levels of spectacular. I dont see anything obvious in there but that doesn't
mean much, I know diddly about perl and its extensions.



I ran it with some added debugging on rpm side which I'm more familiar
with, and the crash happens because a totally garbage pointer is passed to
headerFree(). Well indeed, it was passing the address of the header pointer
variable as the header itself into the callback, and when you try do stuff
with that, well...

This fixes it:

diff --git a/src/RPM4.xs b/src/RPM4.xs
index 04c65ee..6604477 100644
--- a/src/RPM4.xs
+++ b/src/RPM4.xs
@@ -246,7 +246,7 @@ static void *
   s_what = "INST_START";
   if (h) {
   mXPUSHs(newSVpv("header", 0));
-mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
+mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, h));
   #ifdef HDRPMMEM




Oh and you'll want to fix the debug printf too, even though it's obviously
harmless (but then useless for debugging):


   PRINTF_NEW(bless_header, , -1);


   ^^



Blech, one of those days...

The above is closer to mark but still not quite right and will crash too,
only in a different way because the refcount is wrong. Here's the real deal:

diff --git a/src/RPM4.xs b/src/RPM4.xs
index 04c65ee..f7cfd33 100644
--- a/src/RPM4.xs
+++ b/src/RPM4.xs
@@ -246,9 +246,9 @@ static void *
  s_what = "INST_START";
  if (h) {
  mXPUSHs(newSVpv("header", 0));
-mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
+mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
headerLink(h)));
  #ifdef HDRPMMEM
-PRINTF_NEW(bless_header, , -1);
+PRINTF_NEW(bless_header, h, -1);
  #endif
  }
  break;

 - Panu -



Thanks
Now remains the issue with several builds on the same spec object



Yeah, I was just about to write about that.

I totally misunderstood the case initially (that's what happens if you 
try to understand the world by peeking through a keyhole), I thought 
your testcase was looking at the number of files by itself and finding 
one more than expected. When the error actually comes from rpm itself.


That file entries get messed when reusing the same spec is probably an 
ages old issue, it's just that until now rpm didn't sanity check the 
file data.


The right thing to do is to reload the spec, I doubt it ever *really* 
worked correctly.


- Panu -


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

2017-10-05 Thread Thierry Vignaud
On 5 October 2017 at 10:15, Panu Matilainen  wrote:
 Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
 earlier because this already shows it's not a regression in 4.14.x but
 something else. A bug in perl-RPM4 perhaps, as compiling it with -Og makes
 the crash go away, other optimization levels make it blow up with different
 levels of spectacular. I dont see anything obvious in there but that 
 doesn't
 mean much, I know diddly about perl and its extensions.
>>>
>>>
>>> I ran it with some added debugging on rpm side which I'm more familiar
>>> with, and the crash happens because a totally garbage pointer is passed to
>>> headerFree(). Well indeed, it was passing the address of the header pointer
>>> variable as the header itself into the callback, and when you try do stuff
>>> with that, well...
>>>
>>> This fixes it:
>>>
>>> diff --git a/src/RPM4.xs b/src/RPM4.xs
>>> index 04c65ee..6604477 100644
>>> --- a/src/RPM4.xs
>>> +++ b/src/RPM4.xs
>>> @@ -246,7 +246,7 @@ static void *
>>>   s_what = "INST_START";
>>>   if (h) {
>>>   mXPUSHs(newSVpv("header", 0));
>>> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
>>> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, h));
>>>   #ifdef HDRPMMEM
>>
>>
>>
>> Oh and you'll want to fix the debug printf too, even though it's obviously
>> harmless (but then useless for debugging):
>>
>>>   PRINTF_NEW(bless_header, , -1);
>>
>>   ^^
>
>
> Blech, one of those days...
>
> The above is closer to mark but still not quite right and will crash too,
> only in a different way because the refcount is wrong. Here's the real deal:
>
> diff --git a/src/RPM4.xs b/src/RPM4.xs
> index 04c65ee..f7cfd33 100644
> --- a/src/RPM4.xs
> +++ b/src/RPM4.xs
> @@ -246,9 +246,9 @@ static void *
>  s_what = "INST_START";
>  if (h) {
>  mXPUSHs(newSVpv("header", 0));
> -mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, ));
> +mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
> headerLink(h)));
>  #ifdef HDRPMMEM
> -PRINTF_NEW(bless_header, , -1);
> +PRINTF_NEW(bless_header, h, -1);
>  #endif
>  }
>  break;
>
> - Panu -
>

Thanks
Now remains the issue with several builds on the same spec object
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault (was: Re: [Rpm-announce] RPM 4.14.0 release candidate 2 is out)

2017-10-05 Thread Panu Matilainen

On 10/04/2017 06:59 PM, Thierry Vignaud wrote:

On 4 October 2017 at 14:17, Thierry Vignaud  wrote:

Also this new rpm introduced segfault regressions in both RPM4 &
urpmi
testsuites
See attached gdb traces in BUG*.txt
valgrind seems to hint about invalid writes/reads
See you





The urpmi issue is when checking bogus pkgs.
The RPM4 issue is when traversing the transaction (not the rpmdb)
Attached are the valgrind outputs



So we have stuff like

==14087== Invalid write of size 4
==14087==at 0x103AA6DD: headerUnlink (header.c:188)
==14087==by 0x103AA6DD: headerFree (header.c:194)
==14087==by 0xFF69314: XS_RPM4__Header_DESTROY (RPM4.xs:890)
==14087==by 0x3F512E2C40: Perl_pp_entersub (pp_hot.c:4231)
==14087==by 0x3F5125551E: Perl_call_sv (perl.c:2848)
==14087==by 0x3F512E7C09: S_curse (sv.c:6987)
==14087==by 0x3F512E84F7: Perl_sv_clear (sv.c:6591)
==14087==by 0x3F512E898D: Perl_sv_free2 (sv.c:7088)
==14087==by 0x3F513182E6: UnknownInlinedFun (inline.h:200)
==14087==by 0x3F513182E6: Perl_free_tmps (scope.c:212)
==14087==by 0x3F512DAD74: Perl_pp_nextstate (pp_hot.c:52)
==14087==by 0x3F512DAA55: Perl_runops_standard (run.c:41)
==14087==by 0x3F5125D236: S_run_body (perl.c:2524)
==14087==by 0x3F5125D236: perl_run (perl.c:2447)
==14087==by 0x400C79: main (perlmain.c:123)
==14087==  Address 0xffeffef8c is on thread 1's stack
==14087==  396 bytes below stack pointer

...and all the failures are around headerFree(), but none of the
traces
go
into rpm itself, so I dont really know what does "traversing the
transaction" actually mean.




in both URPM & RPM4 bindings, there's a family of traverse functions
that enable to execute a callback on each package of either:
- rpmdb
- the current transaction,
- pkgs header from an hdlist file
- synthetic metadata from a synthesis file
calling the callback on the currrent header

See:
- Db_traverse*() & Trans_traverse() in perl-URPM/URPM.xs
- Ts_traverse() in RPM4-0.36/src/RPM4.xs

It's loosely documented at
http://search.cpan.org/~tvignaud/URPM-5.12/URPM.pm or


http://search.cpan.org/~tvignaud/RPM4-0.36/lib/RPM4/Transaction.pm#Hdlist::Db-%3Etraverse_headers(sub)
(RPM4's doc is very incomplete -- I'm only maintaining this b/c other
tools depend on that and I cannot break them when upgrading rpm)


But the problem is simply with perl-RPM4 and
urpmi passing uninitialized variables to headerFree().

What changed in rpm is that rpmReadPackageFile() no longer does this
as
the
first thing:

   if (hdrp)
   *hdrp = NULL;

Ie if you pass an uninitialized pointer as hdrp, it remains
uninitialized
unless rpmReadPackageFile() returns with a success code.
Which is how I think it should be, but it does deserve a release note
on
the
changed API.

So the moral of the story is basically: if you depend on your
variables
being initialized, initialize them by yourself. It's a good practise
anyway.




I'll check for uninitialized variables later today




Actually the bug happen when running the transaction



Right, that makes sense, there's a second rpmReadPackageFile() inside
transaction run.



Intestingly, perl-RPM4 no more segfaults (there still faillures
though) if I alter the transrun() method (which calls rpmtsRun in
rpmlib) 's perl callback to *not* call std rpmShowProgress())
See attached patch

Aka previously it runs both the passed perl callback + rpmlib default
callback which worked smoothly.
But since rpm-4.14, calling std rpm callback results a segfault...
I'm happy to not call that and I don't think we have any tool actually
running transactions^W installing packages through RPM4 right now but
still that's a regression in rpm...
Maybe some new unlink call or sg like that



Are you saying you still get that segfault with this fix?
https://github.com/rpm-software-management/rpm/commit/082e6e77dd613efa643b02ee0417c1382f520893


That fix was for URPM segfaulting while verifying bad rpms, here I'm
talking about RPM4 segfaulting while installing packages
(different binding, different issue)
The URPM issue is fixed by the above commit (s/is/should/ - I hadn't
actually tested your fix)
The RPM4 issue is a different issue, even if both happen in headerFree() .
You might get confused by the perl gdb traces that looks similar but
the issues really are different.
They show the point where the perl interpreter segfault, but not the
point where the call in rpmlib has garbaged things as this action has
finished and we'd return to the perl interpreter.


 From the traceback you posted it was tripping on the uninitialized header
pointer as the other one. And if you disable the entire callback then there
are no open files returned and the whole error path is different.


There's still a perl callback that's run.
Actually there's a C/XS wrapper (transCallback()), running the
optional pure perl callback if present
Ts_transrun => rpmtsSetNotifyCallback with transCallback wrapper as
callback and the