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

Re: [rfc] leverage -x/-X tar call to pass more tar options



Hi,

Pierre Habouzit wrote:

> The point is that doing chained pipes is pretty annoying from C
> (especially if you want to use parallelism in the process), plus it kind
> of make sense for many kind of operations where you don't need
> --fsys-tarfile anymore: e.g. extracting a single file can be done with:
> 
>     dpkg-deb -x file.deb some-dir ./path/to/file/to/extract

I have wished for something like this before.

> +++ b/dpkg-deb/extract.c
> @@ -325,7 +329,17 @@ void extracthalf(const char *debar, const char *directory,
>  
>        unsetenv("TAR_OPTIONS");
>  
> -      execlp(TAR, "tar", buffer, "-", NULL);
> +      for (i = 0; taropts && taropts[i]; i++);

Produces a warning from gcc.

> +      arg = targv = m_malloc((5 + i + 1) * sizeof(targv[0]));

Why not (4 + i) * sizeof(targv[0])?

> +      *arg++ = "tar";
> +      *arg++ = buffer;
> +      *arg++ = "-";
> +      if (i) {

Needed for the !taropts case.  Good.

> +        mempcpy(arg, taropts, i * sizeof(taropts[0]));
> +        arg += i;
> +      }
> +      *arg++ = NULL;
> +      execvp(TAR, (char *const *)targv);
>        ohshite(_("failed to exec tar"));
>      }
>      close(p2[0]);

I couldn’t find any other uses of mempcpy in dpkg, so it might be better
to use memcpy() or use a wrapper like git does.

> diff --git a/dpkg-deb/main.c b/dpkg-deb/main.c
> index 319e715..8238720 100644
> --- a/dpkg-deb/main.c
> +++ b/dpkg-deb/main.c
> @@ -78,8 +78,10 @@ usage(const struct cmdinfo *cip, const char *value)
>  "  -W|--show <deb>                  Show information on package(s)\n"
>  "  -f|--field <deb> [<cfield> ...]  Show field(s) to stdout.\n"
>  "  -e|--control <deb> [<directory>] Extract control info.\n"
> -"  -x|--extract <deb> <directory>   Extract files.\n"
> -"  -X|--vextract <deb> <directory>  Extract & list files.\n"
> +"  -x|--extract <deb> <directory> [tar-options...]\n"
> +"                                   Extract files.\n"
> +"  -X|--vextract <deb> <directory> [tar-options...]\n"
> +"                                   Extract & list files.\n"
>  "  --fsys-tarfile <deb>             Output filesystem tarfile.\n"
>  "\n"));
>  

Might be worth hinting that myopt will try to consume any tar option
flags itself.  Something like:

	"  -x|--extract <deb> <directory> [[--] tar-options...]\n"
	"                                   Extract files.\n"
	"  -X|--vextract <deb> <directory> [[--] tar-options...]\n"
	"                                   Extract & list files.\n"

> diff --git a/man/dpkg-deb.1 b/man/dpkg-deb.1
> index ec451ef..c5db6e7 100644
> --- a/man/dpkg-deb.1
> +++ b/man/dpkg-deb.1
> @@ -123,9 +123,11 @@ package archive. It is currently produced in the format generated by
>  .BR tar 's
>  verbose listing.
>  .TP
> -.BR \-x ", " \-\-extract " \fIarchive directory\fP"
> +.BR \-x ", " \-\-extract " \fIarchive directory\fP [\fItar-options\fP...]"
>  Extracts the filesystem tree from a package archive into the specified
> -directory.
> +directory. Additionnal arguments are passed straight to
> +.BR tar (1)
> + .

Likewise.

>  
>  Note that extracting a package to the root directory will
>  .I not
> @@ -137,7 +139,7 @@ to install packages.
>  (but not its parents) will be created if necessary, and its permissions
>  modified to match the contents of the package.
>  .TP
> -.BR \-X ", " \-\-vextract " \fIarchive directory\fP"
> +.BR \-X ", " \-\-vextract " \fIarchive directory\fP [\fItar-options\fP...]"
>  Is like
>  .BR \-\-extract " (" \-x ")"
>  but prints a listing of the files extracted as it goes.

Likewise.

With whatever subset of the changes suggested seems suitable,

  Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

diff --git a/dpkg-deb/extract.c b/dpkg-deb/extract.c
index 914224b..2f6ac57 100644
--- a/dpkg-deb/extract.c
+++ b/dpkg-deb/extract.c
@@ -329,13 +329,18 @@ void extracthalf(const char *debar, const char *directory,
 
       unsetenv("TAR_OPTIONS");
 
-      for (i = 0; taropts && taropts[i]; i++);
-      arg = targv = m_malloc((5 + i + 1) * sizeof(targv[0]));
+      for (i = 0; taropts && taropts[i]; i++)
+        ; /* just counting */
+      arg = targv = m_malloc((
+        3 +	/* tar tvf - */
+        i +	/* taropts */
+        1	/* terminating NULL */
+        ) * sizeof(targv[0]));
       *arg++ = "tar";
       *arg++ = buffer;
       *arg++ = "-";
       if (i) {
-        mempcpy(arg, taropts, i * sizeof(taropts[0]));
+        memcpy(arg, taropts, i * sizeof(taropts[0]));
         arg += i;
       }
       *arg++ = NULL;
diff --git a/dpkg-deb/main.c b/dpkg-deb/main.c
index 8238720..3ad20e6 100644
--- a/dpkg-deb/main.c
+++ b/dpkg-deb/main.c
@@ -78,9 +78,9 @@ usage(const struct cmdinfo *cip, const char *value)
 "  -W|--show <deb>                  Show information on package(s)\n"
 "  -f|--field <deb> [<cfield> ...]  Show field(s) to stdout.\n"
 "  -e|--control <deb> [<directory>] Extract control info.\n"
-"  -x|--extract <deb> <directory> [tar-options...]\n"
+"  -x|--extract <deb> <directory> [[--] tar-options...]\n"
 "                                   Extract files.\n"
-"  -X|--vextract <deb> <directory> [tar-options...]\n"
+"  -X|--vextract <deb> <directory> [[--] tar-options...]\n"
 "                                   Extract & list files.\n"
 "  --fsys-tarfile <deb>             Output filesystem tarfile.\n"
 "\n"));
diff --git a/man/dpkg-deb.1 b/man/dpkg-deb.1
index c5db6e7..c267734 100644
--- a/man/dpkg-deb.1
+++ b/man/dpkg-deb.1
@@ -123,11 +123,13 @@ package archive. It is currently produced in the format generated by
 .BR tar 's
 verbose listing.
 .TP
-.BR \-x ", " \-\-extract " \fIarchive directory\fP [\fItar-options\fP...]"
+.BR \-x ", " \-\-extract " \fIarchive directory\fP [[\-\-] \fItar-options\fP...]"
 Extracts the filesystem tree from a package archive into the specified
-directory. Additionnal arguments are passed straight to
-.BR tar (1)
- .
+directory. Any arguments left over after dpkg\-deb processes its own are
+passed to
+.BR tar "(1)."
+Option flags (e.g., \-\-exclude) are only passed to tar if preceded by
+\-\- on the command line.
 
 Note that extracting a package to the root directory will
 .I not
@@ -139,7 +141,7 @@ to install packages.
 (but not its parents) will be created if necessary, and its permissions
 modified to match the contents of the package.
 .TP
-.BR \-X ", " \-\-vextract " \fIarchive directory\fP [\fItar-options\fP...]"
+.BR \-X ", " \-\-vextract " \fIarchive directory\fP [[\-\-] \fItar-options\fP...]"
 Is like
 .BR \-\-extract " (" \-x ")"
 but prints a listing of the files extracted as it goes.
-- 


Reply to: