Re: [PATCH 9/9] xfsdump: fix uninit ackstr in content_mediachange_query()

[Eric Sandeen <sandeen@redhat.com>]

On 11/13/14 12:10 PM, Mark Tinguely wrote:
> On 11/12/14 12:57, Eric Sandeen wrote:
>> Today, this sends an uninitialized ackstr[0] to be mlog'd -
>> who knows what we get out of it.  Other places follow this
>> "count = 0, string = "\n"" pattern which seemsa bit odd, but
>> better than printing uninitialized memory.
>>
>> To be completely honest, I have no test for this.
>>
>> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
>> ---
>>   restore/content.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/restore/content.c b/restore/content.c
>> index c99aed7..bc5b398 100644
>> --- a/restore/content.c
>> +++ b/restore/content.c
>> @@ -2915,6 +2915,7 @@ content_mediachange_query( void )
>>               bagp = 0;
>>           }
>>           ackcnt = 0;
>> +        ackstr[ ackcnt++ ] = "\n";
>>           dlog_multi_ack( ackstr,
>>                   ackcnt );
>>           querycnt = 0;
> 
> dlog_multi_ack() with count of 0 will exit without doing anything.

Oh, yeah, I misread it.  Sorry.  I saw stuff like this in sigint_dialog()

                ackcnt = 0;
                ackstr[ ackcnt++ ] = "\n";
                dlog_multi_ack( ackstr,
                                ackcnt );


and got carried away.

> Looks like some conditional code that filled the ackstr array (like
> the other callers) was removed. I vote to pull ackstr, ackcnt and the
> dlog_multi_ack() from this function.

I wondered about that, but didn't want to go changing things I wasn't
able to test or trivially understand.

However, I guess I'm challenged even in the latter this week.  ;)

Anyway, yeah, just drop this patch, sorry.

Thanks for the review,
-Eric


> --Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs