xfs
[Top] [All Lists]

Re: [PATCH] xfsdump: call mlog_exit in content_stream_restore

To: aelder@xxxxxxx
Subject: Re: [PATCH] xfsdump: call mlog_exit in content_stream_restore
From: Bill Kendall <wkendall@xxxxxxx>
Date: Fri, 02 Sep 2011 08:57:40 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1314375344.2821.47.camel@doink>
References: <1313434763-22340-1-git-send-email-wkendall@xxxxxxx> <1314375344.2821.47.camel@doink>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.18) Gecko/20110617 Thunderbird/3.1.11
On 08/26/2011 11:15 AM, Alex Elder wrote:
On Mon, 2011-08-15 at 13:59 -0500, Bill Kendall wrote:
This patch adds mlog_exit() calls to all the return paths in
content_stream_restore(). mlog_exit() is supposed to be called before
returning from content_stream_dump() and content_stream_restore(), but
many paths in the latter did not do so, allowing for the stream exit
status to be incorrect.

...

The end of this function is a whole bunch of repetitive
code.  It would be cleaner to assign a "ret" variable
(or whatever name you think fits the existing code)
and then after this last switch statement call:

        return mlog_exit(ret, rv);

(If Media_end() got a flag, you might not need the
switch statement at all...)

Christoph suggested a goto which would be similar
but would affect the whole function.  And in fact
I think it might simplify a lot--possibly eliminating
whole switch statements entirely--so I think that's
an idea worth considering.

I looked at doing this, but it didn't result in any switch
statements being removed. Each switch statement is different
enough that we can't just have a single switch at the end
of the function that maps the RV_* value to the proper
EXIT_* value.

I made some other minor changes based on suggestions, and
will repost after the "xfsdump: remove unnecessary" patch
is reviewed. (This patch overlaps a bit with that one.)

Thanks,
Bill

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH] xfsdump: call mlog_exit in content_stream_restore, Bill Kendall <=