On Jun 21, 2008, at 10:32 AM, Denis Washington wrote:

>
> Thanks for taking a look. I am really new to programming RPM, so any
> help is greatly appreciated.
>


>> In general, what is being called a Header does not include sufficient
>> metadata to be casually installed in an rpmdb. You can/will create
>> cause existing rpm deployments to segfault if you proceed with
>> the header as constructed in this implementation.
>
> I already figured that I might not have added all mandatory  
> metadata to
> the header. Unfortunately, I didn't find any documentation on which  
> data
> must be present. Can you point me at the right direction?
>

The tag descriptions in the LSB packaging standard are included
as an appendix to the "RedHat RPM Guide" which is available
somewhere in Fedorable land. The text is also maintained somewhere
within LSB as well.

>> Look closely at the "LSB Packaging" documentation that describes
>> tag content, paying particular attention to the content that LSB has
>> chosen to call "MANDATORY". The fact that the header is in a rpmdb
>> rather than a *.rpm package does not permit MANDATORY to be ignored.
>>
>> Adding both RPMTAG_FILENAMES  and RPMTAG_ 
>> {DIRNAMES,BASENAMES,DIRINDEXES}
>> is just wrong. One or the other should be done, not both.
>
> OK. Again, I didn't find any documentation on this, so I just filled
> both. Which of them is preferred?
>

RPMTAG_FILENAMES was abandoned by RPM in RHL 7.0 ~2000, but is mired in
what is now known as the "LSB format".

You will have to choose which of the conflicting representations for
file paths you wish to use. Including both is clearly a flaw, and worse
than either.

>> The transaction set (and the underlying configuration/rpmdb handling)
>> cannot be scoped within the individual methods if you are going
>> to compute and add RPMTAG_SIZE lazily in _close_package().
>
> I don't know what you mean here. I use a separate transaction set for
> _register_package() and _close_package(), closing after each function.
>

What I mean is that the _register_package() method constructs
what you are calling a Header, and calls rpmdbAdd().

Then the _close_package() method comes along later, reopen's
an rpmdb, adds RPMTAG_SIZE, and rewrites the header.

There's a racy window between your 2 method calls that will lead
to very hard to diagnose issues. Adding RPMTAG_SIZE in
the _register_package() method turns _close_package() into
a no-operation needed stub, and otherwise avoids statefulness.

>> A package header  can change in an rpmdb between method
>> calls, and keeping the rpmdb open (or alternatively, verifying that
>> the header retrieved is the one that should be modified). I see
>> no reason why RPMTAG_SIZE needs to be added in _close_package().
>
> Yeah, I should probably verify that I got the right package in
> _close_package(). Note that when _register_package() is finished,  
> there
> are no files installed yet, only stub files; that's why we can only  
> add
> the files' sizes in _close_package(), after the installer has  
> copied the
> "real" files over. RPMTAG_SIZE could be set to 0 in  
> _register_package(),
> though.
>

Why bother with verify? Just avoid the issue, calculate RPMTAG_SIZE  
when you
have complete information, and add the header when the Header
is complete. Incremental additions to binary Header blobs will
lead to all sorts of issues, including the raciness I pointed out.

>> (aside) There will be other DoS issues that you will encounter if you
>> try
>> to keep an rpmdb (or any database) open persistently in a daemon.
>
> That's why both _register_package() and _close_package() close the DB
> when there finished. The database is just open for while those two
> functions are running.
>
>> There are two calls to
>>      rpmdbSetIteratorModified(iter, 1);
>> one of the calls is unnecessary.
>
> Oh, right. Oops.
>
>> I also strongly encourage you _NOT_ to incrementally add tags to a
>> header with a lazy rewrite into an rpmdb using
>> rpmdbSetIteratorModified().
>
> What is the better way?
>

Build a complete header, call rpmdbAdd() exactly once, removes
an immense amount of statefulness.

Whether the one rpmdbAdd() call is done in _register_package()
or _close_package() matters little. Limiting the time window where
you need exclusive access to an rpmdb is what is needed.

That's generally true for all database applications, not just an rpmdb,
and even more important if/when you are attempting a daemon back end
to a database.
> Regards,
> Denis
>
> P.S.: For follow-ups of general interest, I recommend you to reply  
> on packaging@lists.linux-foundation.org. You can subscribe here:
>
>   https://lists.linux-foundation.org/mailman/listinfo/packaging
>

I will not reply on the packaging list, having received hostile  
threats there
in the past.

73 de Jeff