On 12/17/2009 10:47 AM, Martin Sivak wrote:
> ---
> anaconda.spec.in | 3 +
> loader/Makefile.am | 6 +-
> loader/rpmextract.c | 284 +++++++++++++++++++++++++++++++++++++++++++++++++++
> loader/rpmextract.h | 47 +++++++++
> 4 files changed, 338 insertions(+), 2 deletions(-)
> create mode 100644 loader/rpmextract.c
> create mode 100644 loader/rpmextract.h
>
> diff --git a/anaconda.spec.in b/anaconda.spec.in
> index 862e306..e180345 100644
> --- a/anaconda.spec.in
> +++ b/anaconda.spec.in
> @@ -55,6 +55,7 @@ BuildRequires: gettext >= %{gettextver}
> BuildRequires: gtk2-devel
> BuildRequires: intltool >= %{intltoolver}
> BuildRequires: isomd5sum-devel
> +BuildRequires: libarchive-devel
> BuildRequires: libX11-devel
> BuildRequires: libXt-devel
> BuildRequires: libXxf86misc-devel
> @@ -69,6 +70,7 @@ BuildRequires: pango-devel
> BuildRequires: pykickstart >= %{pykickstartver}
> BuildRequires: python-devel
> BuildRequires: python-urlgrabber
> +BuildRequires: rpm-devel
> BuildRequires: rpm-python >= %{rpmpythonver}
> BuildRequires: slang-devel >= %{slangver}
> BuildRequires: xmlto
> @@ -103,6 +105,7 @@ Requires: device-mapper-libs >= %{dmver}
> Requires: dosfstools
> Requires: e2fsprogs >= %{e2fsver}
> Requires: gzip
> +Requires: libarchive
> %ifarch %{ix86} x86_64 ia64
> Requires: dmidecode
> %endif
> diff --git a/loader/Makefile.am b/loader/Makefile.am
> index e5d1b7f..787fdc8 100644
> --- a/loader/Makefile.am
> +++ b/loader/Makefile.am
> @@ -45,13 +45,15 @@ loader_CFLAGS = $(COMMON_CFLAGS) $(GLIB_CFLAGS) $(LIBNM_GLIB_CFLAGS) \
> loader_LDADD = $(NEWT_LIBS) $(GLIB_LIBS) $(LIBNL_LIBS) \
> $(LIBNM_GLIB_LIBS) $(CHECKISOMD5_LIBS) \
> $(LIBCURL_LIBS) \
> - $(ISCSI_LIBS) $(top_srcdir)/isys/libisys.la
> + $(ISCSI_LIBS) $(top_srcdir)/isys/libisys.la \
> + -lrpm -lrpmio -larchive
> loader_SOURCES = loader.c copy.c log.c moduleinfo.c loadermisc.c \
> modules.c windows.c lang.c kbd.c driverdisk.c \
> selinux.c mediacheck.c kickstart.c driverselect.c \
> getparts.c dirbrowser.c fwloader.c ibft.c hardware.c \
> method.c cdinstall.c hdinstall.c nfsinstall.c \
> - urlinstall.c net.c urls.c telnet.c telnetd.c
> + urlinstall.c net.c urls.c telnet.c telnetd.c \
> + rpmextract.c
The indentation here is weird.
> init_CFLAGS = $(COMMON_CFLAGS)
> init_SOURCES = init.c undomounts.c shutdown.c copy.c
> diff --git a/loader/rpmextract.c b/loader/rpmextract.c
> new file mode 100644
> index 0000000..44c0890
> --- /dev/null
> +++ b/loader/rpmextract.c
> @@ -0,0 +1,284 @@
> +/* rpm2dir: unpack the payload of RPM package to the current directory*/
> +/* Based on rpm2cpio */
> +
Copyright/gpl blob here would be good.
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <rpm/rpmlib.h> /* rpmReadPackageFile .. */
> +#include <rpm/rpmtag.h>
> +#include <rpm/rpmio.h>
> +#include <rpm/rpmpgp.h>
> +
> +#include <rpm/rpmts.h>
> +
> +#include <stdio.h>
> +#include <archive.h>
> +#include <archive_entry.h>
> +
> +#include "loader.h"
> +#include "log.h"
> +#include "rpmextract.h"
> +
> +/*
> + * internal structure to pass to libarchive callbacks
> + */
> +
> +struct cpio_mydata {
> + FD_t gzdi;
> + char *buffer;
> +};
> +
> +/*
> + * libarchive callbacks
> + */
> +
> +ssize_t rpm_myread(struct archive *a, void *client_data, const void **buff)
> +{
> + struct cpio_mydata *mydata = client_data;
> + *buff = mydata->buffer;
> + return (Fread(mydata->buffer, BUFFERSIZE, 1, mydata->gzdi));
Return shouldn't have parens. Also you've mixed two and four space indentation
in this new file.
> +}
> +
> +int rpm_myclose(struct archive *a, void *client_data)
> +{
> + struct cpio_mydata *mydata = client_data;
> + if (mydata->gzdi > 0)
> + Fclose(mydata->gzdi);
> + return (ARCHIVE_OK);
More with the weird parentheses.
> +}
> +
> +/* read data from RPM header */
> +
> +const char * headerGetString(Header h, rpmTag tag)
> +{
> + const char *res = NULL;
> + struct rpmtd_s td;
> +
> + if (headerGet(h, tag, &td, HEADERGET_MINMEM)) {
> + if (rpmtdCount(&td) == 1) {
> + res = rpmtdGetString(&td);
> + }
> + rpmtdFreeData(&td);
> + }
> + return res;
> +}
> +
> +/*
> + * explode source RPM into the current directory
> + * use filters to skip packages and files we do not need
> + */
> +int explodeRPM(const char *source,
> + filterfunc filter,
> + dependencyfunc provides,
> + dependencyfunc deps,
> + void* userptr)
> +{
> + char buffer[BUFFERSIZE+1]; /* make space for trailing \0 */
> + FD_t fdi;
> + Header h;
> + char * rpmio_flags = NULL;
> + rpmRC rc;
> + FD_t gzdi;
> + struct archive *cpio;
> + struct archive_entry *cpio_entry;
> + struct cpio_mydata cpio_mydata;
> +
> + if (strcmp(source, "-") == 0)
> + fdi = fdDup(STDIN_FILENO);
> + else
> + fdi = Fopen(source, "r.ufdio");
> +
> + if (Ferror(fdi)) {
> + logMessage(ERROR, "%s: %s\n", (strcmp(source, "-") == 0 ? "<stdin>" : source), Fstrerror(fdi));
Using a temporary variable here would make this easier to read.
> + return EXIT_FAILURE;
> + }
> + rpmReadConfigFiles(NULL, NULL);
> +
> + {
Please no naked blocks ever.
> + rpmts ts = rpmtsCreate();
> + rpmVSFlags vsflags = 0;
> +
> + /* Do not check digests, signatures or headers */
> + vsflags |= _RPMVSF_NODIGESTS;
> + vsflags |= _RPMVSF_NOSIGNATURES;
> + vsflags |= RPMVSF_NOHDRCHK;
> + (void) rpmtsSetVSFlags(ts, vsflags);
> +
> + rc = rpmReadPackageFile(ts, fdi, "rpm2dir", &h);
> +
> + ts = rpmtsFree(ts);
> + }
> +
> + switch (rc) {
> + case RPMRC_OK:
> + case RPMRC_NOKEY:
> + case RPMRC_NOTTRUSTED:
> + break;
> + case RPMRC_NOTFOUND:
> + logMessage(ERROR, "%s is not an RPM package", source);
> + return EXIT_FAILURE;
> + break;
> + case RPMRC_FAIL:
> + default:
> + logMessage(ERROR, "error reading header from %s package\n", source);
> + return EXIT_FAILURE;
> + break;
> + }
> +
> + /* Retrieve all dependencies and run them through deps function */
> + while (deps){
A space between ) and { would be good.
> + struct rpmtd_s td;
> + const char *depname;
> +
> + if (!headerGet(h, RPMTAG_REQUIRENAME, &td, HEADERGET_MINMEM))
> + break;
> +
> + /* iterator */
> + while ((depname = rpmtdNextString(&td))) {
> + if (deps(depname, userptr)) {
> + Fclose(fdi);
> + return EXIT_BADDEPS;
> + }
> + }
> + rpmtdFreeData(&td);
> + break;
> + }
> +
> + /* Retrieve all provides and run them through provides function */
> + while (provides){
And again...
> + struct rpmtd_s td;
> + const char *depname;
> + int found = 0;
> +
> + if (!headerGet(h, RPMTAG_PROVIDES, &td, HEADERGET_MINMEM))
> + break;
> +
> + /* iterator */
> + while ((depname = rpmtdNextString(&td))) {
> + if (!provides(depname, userptr)) {
> + found++;
> + }
> + }
> + rpmtdFreeData(&td);
> + if (found<=0)
> + return EXIT_BADDEPS;
> + break;
> + }
> +
> + /* Retrieve type of payload compression. */
> + {
> + const char *compr = headerGetString(h, RPMTAG_PAYLOADCOMPRESSOR);
> + if (compr && strcmp(compr, "gzip")) {
> + checked_asprintf(&rpmio_flags, "r.%sdio", compr);
> + }
> + else {
> + checked_asprintf(&rpmio_flags, "r.gzdio");
> + }
> + }
Please no naked blocks ever.
> +
> + /* Open uncompressed cpio stream */
> + gzdi = Fdopen(fdi, rpmio_flags);
> + free(rpmio_flags);
> +
> + if (gzdi == NULL) {
> + logMessage(ERROR, "cannot re-open payload: %s\n", Fstrerror(gzdi));
> + return EXIT_FAILURE;
> + }
> +
> + /* initialize cpio decompressor */
> + cpio = archive_read_new();
> + if (cpio==NULL) {
> + Fclose(gzdi);
> + return -1;
> + }
> +
> + cpio_mydata.gzdi = gzdi;
> + cpio_mydata.buffer = buffer;
> + archive_read_support_compression_all(cpio);
> + archive_read_support_format_all(cpio);
> + rc = archive_read_open(cpio, &cpio_mydata, NULL, rpm_myread, rpm_myclose);
> +
> + /* read all files in cpio archive */
> + while (rc==ARCHIVE_OK && (rc = archive_read_next_header(cpio, &cpio_entry)) == ARCHIVE_OK){
This should be more like:
if (rc != ARCHIVE_OK)
return rc;
while ((rc = archive_read_next_header(cpio, &cpio_entry)) == ARCHIVE_OK) {
As it is now, if archive_read_open() fails, you're calling archive_read_finish()
when the open has failed.
> + const struct stat *fstat;
> + int64_t fsize;
> + const char* filename;
> + int needskip = 1; /* do we need to read the data to get to the next header? */
> + int offset = 0;
> + int towrite = 0;
> +
> + filename = archive_entry_pathname(cpio_entry);
> + fstat = archive_entry_stat(cpio_entry);
> + fsize = archive_entry_size(cpio_entry);
> +
> + /* Strip leading slashes */
> + while (filename[offset] == '/') offset+=1;
> + /* Strip leading ./ */
> + while (filename[offset] == '.' && filename[offset+1] == '/') offset+=2;
1) please don't collapse code onto the same line as a while() or if()
2) if "/" or "./" or "." makes it into the cpio ball here, which would be
unfortunate but is possible, then this code segfaults.
> +
> + /* Other file type - we do not care except special cases */
> + if (!S_ISREG(fstat->st_mode)) towrite = 1;
> + else towrite = 2;
Again, the formatting here is bad.
> +
> + if (filter && filter(filename+offset, fstat, userptr)) {
> + /* filter this file */
> + towrite = 0;
> + }
> +
> + /* Create directories */
> + char* dirname = strdup(filename+offset);
This needs error checking.
> + char* dirptr = dirname;
> + while (dirptr!=NULL && *dirptr!=0){
Formatting again. In general, though, this can be more clearly written as:
while (dirptr && *dirptr) {
> + dirptr = strchr(dirptr, '/');
> + if (dirptr) {
> + *dirptr = 0;
> + mkdir(dirname, 0700);
> + *dirptr = '/';
> + dirptr++;
> + }
> + }
> + free(dirname);
> +
> + /* Regular file */
> + if (towrite>=2){
More with the formatting. "if (towrite >= 2) {" is preferred.
> + FILE *fdout = fopen(filename+offset, "w");
> +
> + if (fdout==NULL){
...
> + free((void*)filename);
(void*) shouldn't be here.
> + rc = 33;
> + break;
> + } else if ((rc = archive_read_data_into_fd(cpio, fileno(fdout)))!=ARCHIVE_OK) {
> + /* XXX We didn't get the file.. well.. */
> + needskip = 0;
Since you had a break in the previous bit, this can more clearly be written as:
rc = 33;
break;
}
rc = archive_read_data_into_fd(cpio, fileno(fdout));
if (rc != ARCHIVE_OK) {
/* XXX We didn't get the file.. well.. */
needskip = 0;
And then it fits on a page and doesn't have tons of syntax getting in the way of
reading it.
> + } else {
> + needskip = 0;
> + fclose(fdout);
> + }
> + }
> +
> + /* symlink, we assume that the path contained in symlink
> + * is shorter than BUFFERSIZE */
> + while (towrite && S_ISLNK(fstat->st_mode)) {
> + char symlinkbuffer[BUFFERSIZE-1];
> +
> + needskip = 0;
> + if ((rc = archive_read_data(cpio, symlinkbuffer, fsize))!=ARCHIVE_OK) {
> + /* XXX We didn't get the file.. well.. */
> + break;
> + }
> +
> + if (symlink(buffer, filename+offset)) {
> + /* TODO: error handling */
Obviously this needs to be written before this goes in.
> + }
> +
> + break;
> + }
> +
> + if(needskip) archive_read_data_skip(cpio);
Again with the formatting ;)
> + }
> +
> + archive_read_finish(cpio);
> +
> + return rc!=ARCHIVE_OK;
Comparitors should have spaces.
> +}
> diff --git a/loader/rpmextract.h b/loader/rpmextract.h
> new file mode 100644
> index 0000000..b4624c2
> --- /dev/null
> +++ b/loader/rpmextract.h
> @@ -0,0 +1,47 @@
> +/*
> + File name: rpmextract.h
> + Date: 2009/09/16
> + Author: msivak
> +
> + Copyright (C) 2009 msivak
I think this should say "Copyright 2009 Red Hat, Inc."
> +
> + This program is free software; you can redistribute it and/or
> + modify it under the terms of the GNU General Public License as
> + published by the Free Software Foundation; either version 2 of the
> + License, or (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + in a file called COPYING along with this program; if not, write to
> + the Free Software Foundation, Inc., 675 Mass Ave, Cambridge, MA
> + 02139, USA.
Probably using a GPL template that's less than 10 years old would be better;
FSF hasn't been on Mass Ave in quite some time. Replace the 3rd block with:
You should have received a copy of the GNU Lesser General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +
> +#ifndef __RPMEXTRACT_H__
> +#define __RPMEXTRACT_H__
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +
> +#define EXIT_BADDEPS 4
> +#define BUFFERSIZE 1024
> +
> +/* both filter functions return 0 - match, 1 - match not found */
> +typedef int (*filterfunc)(const char* name, const struct stat *fstat, void *userptr);
> +typedef int (*dependencyfunc)(const char* depends, void *userptr);
> +
> +int explodeRPM(const char* file,
> + filterfunc filter,
> + dependencyfunc provides,
> + dependencyfunc deps,
> + void* userptr);
> +
> +#endif
> +
> +/* end of rpmextract.h */
This looks fine.
--
Peter
I'd like to start a religion. That's where the money is.
-- L. Ron Hubbard to Lloyd Eshbach, in 1949;
quoted by Eshbach in _Over My Shoulder_.
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
