[PATCH 1/1] handle e2image offset value better

[PATCH 1/1] handle e2image offset value better

[jon ernst]

current e2image cannot handle offset value as 0.

For example,
e2image -aro   0 /dev/sda7 ~/e2image7
will return usage()
but
e2image -aro   1 /dev/sda7 ~/e2image7
is fine.


Signed-off-by: "Jon Ernst" <jonernst07@gmail.com>
---
 misc/e2image.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/misc/e2image.c b/misc/e2image.c
index 74515c8..af97d7a 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -1485,9 +1485,11 @@ int main (int argc, char ** argv)
             break;
         case 'o':
             source_offset = strtoull(optarg, NULL, 0);
+            move_mode = 1;
             break;
         case 'O':
             dest_offset = strtoull(optarg, NULL, 0);
+            move_mode = 1;
             break;
         case 'p':
             show_progress = 1;
@@ -1498,10 +1500,7 @@ int main (int argc, char ** argv)
         default:
             usage();
         }
-    if (optind == argc - 1 &&
-        (source_offset || dest_offset))
-            move_mode = 1;
-    else if (optind != argc - 2 )
+    if ((optind != argc - 2 ) && (move_mode))
         usage();

     if (all_data && !img_type) {
-- 
1.8.3.2

Re: [1/1] handle e2image offset value better

[Theodore Ts'o]

On Sun, Jan 05, 2014 at 04:42:42AM +0000, jon ernst wrote:
> current e2image cannot handle offset value as 0.
> 
> For example,
> e2image -aro   0 /dev/sda7 ~/e2image7
> will return usage()
> but
> e2image -aro   1 /dev/sda7 ~/e2image7
> is fine.

I'm not seeing a problem;

    cp /dev/null /tmp/foo.img
    mke2fs -t ext4 -F /tmp/foo.img 100
    e2image -aro 0 /tmp/foo.img /tmp/bar.img

Is working just fine for me.

Looking at your patch, I don't think it's correct.  We only want to go
into "move mode" when the user has specified a single argument, and
either the source or destination is non-zero, and of course this
really only makes sense when we are in raw mode.

That's what the code is currently doing.  If I were going to make any
changes, I'd probably change the criteria so that we check to see if
the source and destination offset is identical (since then it's just a
no-op), and we should probably enforce the restriction that we only
allow move mode when the user has specified both the -a and the -r
option.  (Otherwise, they will destroy the data on their file system,
which is probably not the result they were looking for.)

Regards,

						- Ted

Re: [1/1] handle e2image offset value better

[jon ernst]

On Sat, Jan 11, 2014 at 6:43 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Sun, Jan 05, 2014 at 04:42:42AM +0000, jon ernst wrote:
>> current e2image cannot handle offset value as 0.
>>
>> For example,
>> e2image -aro   0 /dev/sda7 ~/e2image7
>> will return usage()
>> but
>> e2image -aro   1 /dev/sda7 ~/e2image7
>> is fine.
>
> I'm not seeing a problem;
>
>     cp /dev/null /tmp/foo.img
>     mke2fs -t ext4 -F /tmp/foo.img 100
>     e2image -aro 0 /tmp/foo.img /tmp/bar.img
>
Yes, agree. Actually I meant to send test like this:

e2image -aro 0 /tmp/foo.img

This will return usage(). As you mentioned, only "non-zero" argument
makes sense. I agree too.





> Is working just fine for me.
>
> Looking at your patch, I don't think it's correct.  We only want to go
> into "move mode" when the user has specified a single argument, and
> either the source or destination is non-zero, and of course this
> really only makes sense when we are in raw mode.
>
> That's what the code is currently doing.  If I were going to make any
> changes, I'd probably change the criteria so that we check to see if
> the source and destination offset is identical (since then it's just a
> no-op), and we should probably enforce the restriction that we only
> allow move mode when the user has specified both the -a and the -r
> option.  (Otherwise, they will destroy the data on their file system,
> which is probably not the result they were looking for.)

I tested, this restriction is there.

Thank you for your response.
Sorry for useless patch.

Jon
>
> Regards,
>
>                                                 - Ted