Re: [PATCH 2/7] Add rpm extraction routines (use librpm and libarchive)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux