This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: Add ar --output
Hi Fangrui,
> I think that the --output option should keep the is_valid_archive_path
> check.
Ok, I will leave that it in. If users complain in the future then we can
revisit this decision.
> I sent a patch a few days ago:
> https://sourceware.org/ml/binutils/2019-10/msg00193.html (both absolute
Oops, sorry, I missed that.
So I have taken your patch, added a few tweaks of my own, and checked in
the following:
Cheers
Nick
binutils/ChangeLog
2019-10-30 Fangrui Song <i@maskray.me>
Nick Clifton <nickc@redhat.com>
* ar.c (emum long option numbers): Declare. Use to provide
numerical values for long options.
(long_options): Add --output option.
(usage): Mention the --output option.
(open_output_file): New function. Create a filepath for an output
file and open it.
(extract_file): Use open_output_file().
(open_output_file):
* testsuite/binutils-all/ar.exp: Add a test of the new feature.
* doc/binutils.texi: Document the new feature.
* NEWS: Mention the new feature.
diff --git a/binutils/NEWS b/binutils/NEWS
index e6afc4ba29..fd14d71ce9 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,8 @@
-*- text -*-
+* Add --output option to the "ar" program. This option can be used to specify
+ the output directory when extracting members from an archive.
+
Changes in 2.33:
* Add --source-comment[=<txt>] option to objdump which if present,
diff --git a/binutils/ar.c b/binutils/ar.c
index 38c54c9fa8..0af7954a47 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -35,6 +35,7 @@
#include "binemul.h"
#include "plugin-api.h"
#include "plugin.h"
+#include "ansidecl.h"
#ifdef __GO32___
#define EXT_NAME_LEN 3 /* Bufflen of addition to name if it's MS-DOS. */
@@ -149,8 +150,14 @@ static const char *plugin_target = NULL;
static const char *target = NULL;
-#define OPTION_PLUGIN 201
-#define OPTION_TARGET 202
+enum long_option_numbers
+{
+ OPTION_PLUGIN = 201,
+ OPTION_TARGET,
+ OPTION_OUTPUT
+};
+
+static const char * output_dir = NULL;
static struct option long_options[] =
{
@@ -158,6 +165,7 @@ static struct option long_options[] =
{"plugin", required_argument, NULL, OPTION_PLUGIN},
{"target", required_argument, NULL, OPTION_TARGET},
{"version", no_argument, &show_version, 1},
+ {"output", required_argument, NULL, OPTION_OUTPUT},
{NULL, no_argument, NULL, 0}
};
@@ -327,6 +335,7 @@ usage (int help)
fprintf (s, _(" [V] - display the version number\n"));
fprintf (s, _(" @<file> - read options from <file>\n"));
fprintf (s, _(" --target=BFDNAME - specify the target object format as BFDNAME\n"));
+ fprintf (s, _(" --output=DIRNAME - specify the output directory for extraction operations\n"));
#if BFD_SUPPORTS_PLUGINS
fprintf (s, _(" optional:\n"));
fprintf (s, _(" --plugin <p> - load the specified plugin\n"));
@@ -592,6 +601,9 @@ decode_options (int argc, char **argv)
case OPTION_TARGET:
target = optarg;
break;
+ case OPTION_OUTPUT:
+ output_dir = optarg;
+ break;
case 0: /* A long option that just sets a flag. */
break;
default:
@@ -1050,6 +1062,49 @@ print_contents (bfd *abfd)
free (cbuf);
}
+
+static FILE * open_output_file (bfd *) ATTRIBUTE_RETURNS_NONNULL;
+
+static FILE *
+open_output_file (bfd * abfd)
+{
+ output_filename = bfd_get_filename (abfd);
+
+ if (output_dir)
+ {
+ size_t len = strlen (output_dir);
+
+ if (len > 0)
+ {
+ /* FIXME: There is a memory leak here, but it is not serious. */
+ if (IS_DIR_SEPARATOR (output_dir [len - 1]))
+ output_filename = concat (output_dir, output_filename, NULL);
+ else
+ output_filename = concat (output_dir, "/", output_filename, NULL);
+ }
+ }
+
+ /* PR binutils/17533: Do not allow directory traversal
+ outside of the current directory tree. */
+ if (! is_valid_archive_path (output_filename))
+ {
+ char * base = (char *) lbasename (output_filename);
+
+ non_fatal (_("illegal output pathname for archive member: %s, using '%s' instead"),
+ output_filename, base);
+ output_filename = base;
+ }
+
+ FILE * ostream = fopen (output_filename, FOPEN_WB);
+ if (ostream == NULL)
+ {
+ perror (output_filename);
+ xexit (1);
+ }
+
+ return ostream;
+}
+
/* Extract a member of the archive into its own file.
We defer opening the new file until after we have read a BUFSIZ chunk of the
@@ -1063,23 +1118,9 @@ print_contents (bfd *abfd)
void
extract_file (bfd *abfd)
{
- FILE *ostream;
- char *cbuf = (char *) xmalloc (BUFSIZE);
- bfd_size_type nread, tocopy;
- bfd_size_type ncopied = 0;
bfd_size_type size;
struct stat buf;
- /* PR binutils/17533: Do not allow directory traversal
- outside of the current directory tree. */
- if (! is_valid_archive_path (bfd_get_filename (abfd)))
- {
- non_fatal (_("illegal pathname found in archive member: %s"),
- bfd_get_filename (abfd));
- free (cbuf);
- return;
- }
-
if (bfd_stat_arch_elt (abfd, &buf) != 0)
/* xgettext:c-format */
fatal (_("internal stat error on %s"), bfd_get_filename (abfd));
@@ -1090,75 +1131,61 @@ extract_file (bfd *abfd)
bfd_seek (abfd, (file_ptr) 0, SEEK_SET);
- ostream = NULL;
+ output_file = NULL;
if (size == 0)
{
- /* Seems like an abstraction violation, eh? Well it's OK! */
- output_filename = bfd_get_filename (abfd);
+ output_file = open_output_file (abfd);
+ }
+ else
+ {
+ bfd_size_type ncopied = 0;
+ char *cbuf = (char *) xmalloc (BUFSIZE);
- ostream = fopen (bfd_get_filename (abfd), FOPEN_WB);
- if (ostream == NULL)
+ while (ncopied < size)
{
- perror (bfd_get_filename (abfd));
- xexit (1);
- }
+ bfd_size_type nread, tocopy;
- output_file = ostream;
- }
- else
- while (ncopied < size)
- {
- tocopy = size - ncopied;
- if (tocopy > BUFSIZE)
- tocopy = BUFSIZE;
-
- nread = bfd_bread (cbuf, tocopy, abfd);
- if (nread != tocopy)
- /* xgettext:c-format */
- fatal (_("%s is not a valid archive"),
- bfd_get_filename (abfd->my_archive));
+ tocopy = size - ncopied;
+ if (tocopy > BUFSIZE)
+ tocopy = BUFSIZE;
- /* See comment above; this saves disk arm motion */
- if (ostream == NULL)
- {
- /* Seems like an abstraction violation, eh? Well it's OK! */
- output_filename = bfd_get_filename (abfd);
+ nread = bfd_bread (cbuf, tocopy, abfd);
+ if (nread != tocopy)
+ /* xgettext:c-format */
+ fatal (_("%s is not a valid archive"),
+ bfd_get_filename (abfd->my_archive));
- ostream = fopen (bfd_get_filename (abfd), FOPEN_WB);
- if (ostream == NULL)
- {
- perror (bfd_get_filename (abfd));
- xexit (1);
- }
+ /* See comment above; this saves disk arm motion. */
+ if (output_file == NULL)
+ output_file = open_output_file (abfd);
- output_file = ostream;
- }
+ /* fwrite in mingw32 may return int instead of bfd_size_type. Cast
+ the return value to bfd_size_type to avoid comparison between
+ signed and unsigned values. */
+ if ((bfd_size_type) fwrite (cbuf, 1, nread, output_file) != nread)
+ fatal ("%s: %s", output_filename, strerror (errno));
- /* fwrite in mingw32 may return int instead of bfd_size_type. Cast
- the return value to bfd_size_type to avoid comparison between
- signed and unsigned values. */
- if ((bfd_size_type) fwrite (cbuf, 1, nread, ostream) != nread)
- fatal ("%s: %s", output_filename, strerror (errno));
- ncopied += tocopy;
- }
+ ncopied += tocopy;
+ }
- if (ostream != NULL)
- fclose (ostream);
+ free (cbuf);
+ }
+
+ fclose (output_file);
output_file = NULL;
- output_filename = NULL;
- chmod (bfd_get_filename (abfd), buf.st_mode);
+ chmod (output_filename, buf.st_mode);
if (preserve_dates)
{
/* Set access time to modification time. Only st_mtime is
initialized by bfd_stat_arch_elt. */
buf.st_atime = buf.st_mtime;
- set_times (bfd_get_filename (abfd), &buf);
+ set_times (output_filename, &buf);
}
- free (cbuf);
+ output_filename = NULL;
}
static void
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 2edd7e1aa1..97abf980ba 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -169,7 +169,7 @@ in the section entitled ``GNU Free Documentation License''.
@c man title ar create, modify, and extract from archives
@smallexample
-ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [-]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
ar -M [ <mri-script ]
@end smallexample
@@ -253,7 +253,7 @@ program.
@smallexample
@c man begin SYNOPSIS ar
-ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
+ar [@option{-X32_64}] [@option{-}]@var{p}[@var{mod}] [@option{--plugin} @var{name}] [@option{--target} @var{bfdname}] [@option{--output} @var{dirname}] [@var{relpos}] [@var{count}] @var{archive} [@var{member}@dots{}]
@c man end
@end smallexample
@@ -579,6 +579,21 @@ The optional command-line switch @option{--target @var{bfdname}}
specifies that the archive members are in an object code format
different from your system's default format. See
@xref{Target Selection}, for more information.
+
+@item --output @var{dirname}
+The @option{--output} option can be used to specify a path to a
+directory into which archive members should be extracted. If this
+option is not specified then the current directory will be used.
+
+Note - although the presence of this option does imply a @option{x}
+extraction operation that option must still be included on the command
+line.
+
+Note - using this option does not allow archive members to be
+extracted to locations outside of the current directory, or one of its
+sub-directories. This is a security feature to prevent archives
+created with the @option{P} option from maliciously overwriting user
+files.
@end table
@c man end
diff --git a/binutils/testsuite/binutils-all/ar.exp b/binutils/testsuite/binutils-all/ar.exp
index bc1fbdb3cc..c043f7e550 100644
--- a/binutils/testsuite/binutils-all/ar.exp
+++ b/binutils/testsuite/binutils-all/ar.exp
@@ -605,6 +605,50 @@ proc empty_archive { } {
pass $testname
}
+# Test extracting an element.
+
+proc extract_an_element { } {
+ global AR
+ global AS
+ global srcdir
+ global subdir
+
+ set testname "ar extracting an element"
+
+ if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.o] {
+ unresolved $testname
+ return
+ }
+
+ set archive artest.a
+
+ if [is_remote host] {
+ set objfile [remote_download host tmpdir/bintest.o]
+ remote_file host delete $archive
+ } else {
+ set objfile tmpdir/bintest.o
+ }
+
+ remote_file build delete $archive
+
+ set got [binutils_run $AR "-r -c $archive ${objfile}"]
+ if ![string match "" $got] {
+ fail $testname
+ return
+ }
+
+ set got [binutils_run $AR "--output=tmpdir -x $archive ${objfile}"]
+ if ![string match "" $got] {
+ fail $testname
+ return
+ }
+
+ remote_file build delete $archive
+ remote_file build delete tmpdir/$archive
+
+ pass $testname
+}
+
# Run the tests.
# Only run the bfdtest checks if the programs exist. Since these
@@ -625,6 +669,7 @@ deterministic_archive
delete_an_element
move_an_element
empty_archive
+extract_an_element
if { [is_elf_format] && [supports_gnu_unique] } {
unique_symbol