Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rpm2archive behavior with multiple arguments #681

Closed

Conversation

frozencemetery
Copy link
Contributor

If multiple arguments are passed to rpm2archive, convert them all to
tgz. (Previous behavior was to convert only the first one and
silently ignore the rest.)

Signed-off-by: Robbie Harwood rharwood@redhat.com

@ignatenkobrain
Copy link
Contributor

It seems that your patch is broken:

./rpmgeneral.at:318:

runroot rpm2archive - < "${RPMTEST}"/data/RPMS/hello-2.0-1.x86_64.rpm | tar tz

runroot rpm2archive - < "${RPMTEST}"/data/SRPMS/hello-1.0-1.src.rpm | tar tz



--- /dev/null	2019-04-24 19:34:18.132790957 +0000

+++ /opt/rpm/tests/rpmtests.dir/at-groups/6/stderr	2019-04-24 19:34:23.620810286 +0000

@@ -0,0 +1,2 @@

+rpm2archive: --define: No such file or directory

+rpm2archive: --define: No such file or directory

6. rpmgeneral.at:316: 6. rpm2archive (rpmgeneral.at:316): FAILED (rpmgeneral.at:318)


ts = rpmtsFree(ts);
if (argc == 1)
argv[argc++] = "-";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. You can't just write beyond the bounds of the argv array. You need to allocate a new - bigger - one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per C standard, argv[argc] is always defined to be a NULL pointer. (Section 5.1.2.2.1, paragraph 2, in ISO/IEC 9899:1999).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you may not like that I'm doing this, which is a perfectly valid complaint and I'm happy to change it :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't like that. Please change :)

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
If multiple arguments are passed to rpm2archive, convert them all to
tgz.  (Previous behavior was to convert only the first one and
silently ignore the rest.)

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
@frozencemetery
Copy link
Contributor Author

Problem was due to a CI bug, which I've included a fix for.

vsflags |= RPMVSF_MASK_NOSIGNATURES;
vsflags |= RPMVSF_NOHDRCHK;
(void) rpmtsSetVSFlags(ts, vsflags);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this transaction fiddling should be done exactly once, outside the loop.

@ffesti
Copy link
Contributor

ffesti commented Apr 30, 2019

Pushed with minor tweaks. Thanks for the patch and the additional effort of fixing the test suite!

@ffesti ffesti closed this Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants