xfs
[Top] [All Lists]

Re: [PATCH] xfsdump: fix bug restricting -f pathname length

To: aelder@xxxxxxx
Subject: Re: [PATCH] xfsdump: fix bug restricting -f pathname length
From: Bill Kendall <wkendall@xxxxxxx>
Date: Fri, 25 Feb 2011 13:55:22 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1298662057.1990.7054.camel@doink>
References: <1298572850-15408-1-git-send-email-wkendall@xxxxxxx> <1298662057.1990.7054.camel@doink>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Thunderbird/3.1.7
On 02/25/2011 01:27 PM, Alex Elder wrote:
On Thu, 2011-02-24 at 12:40 -0600, Bill Kendall wrote:
A temporary buffer storing the dump filename (-f) is smaller than
the otherwise allowed pathname length. This buffer is used to
extract the pathname from additional parameters specified as part
of the -f argument. As these parameters are an undocumented and
unused extension of the -f argument, simply remove this code.
Note that drive_alloc() always dups the given pathname, so
'optarg' is passed directly.

If someone happens to put something starting with a comma
after the device path name, will it simply fail because
a device by that name (comma included) won't exist?

xfsdump creates the file if it does not exist, so you would
end up with a comma in the dump filename.

Thanks,
Bill


Either way, this looks good.  I'll hold off a bit before
taking this in, in case someone else wants a chance to
review.

Reviewed-by: Alex Elder<aelder@xxxxxxx>

Signed-off-by: Bill Kendall<wkendall@xxxxxxx>
---
  common/drive.c |   18 +-----------------
  1 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/common/drive.c b/common/drive.c
index f184afa..227fecd 100644
--- a/common/drive.c
+++ b/common/drive.c
@@ -137,10 +137,6 @@ drive_init1( int argc, char *argv[ ], bool_t 
singlethreaded )
        opterr = 0;
        driveix = 0;
        while ( ( c = getopt( argc, argv, GETOPT_CMDSTRING )) != EOF ) {
-               char optarray[100];
-               char *devname;
-               char *token;
-
                switch ( c ) {
                case GETOPT_DUMPDEST:
                        if ( ! optarg || optarg[ 0 ] == '-' ) {
@@ -151,21 +147,9 @@ drive_init1( int argc, char *argv[ ], bool_t 
singlethreaded )
                                return BOOL_FALSE;
                        }

-                       /* remove the device name from the rest of the
-                        * parameter string. note that strdup malloc()s
-                        * a string; important since optarray is an auto.
-                        */
-                       ASSERT( strlen( optarg )<  sizeof( optarray ));
-                       strncpy( optarray, optarg, sizeof( optarray ));
-                       optarray[ sizeof( optarray ) - 1 ] = 0;
-                       if ( ( token = strtok( optarray, "," )) == NULL ) {
-                               token = optarray;
-                       }
-                       devname = strdup( token );
-
                        /* allocate a drive descriptor
                         */
-                       drivepp[ driveix ] = drive_alloc( devname, driveix );
+                       drivepp[ driveix ] = drive_alloc( optarg, driveix );
                        driveix++;
                        break;
                }



<Prev in Thread] Current Thread [Next in Thread>