From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60859) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtYcI-0003tR-U0 for qemu-devel@nongnu.org; Wed, 26 Nov 2014 04:10:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtYcD-0006XK-FE for qemu-devel@nongnu.org; Wed, 26 Nov 2014 04:10:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtYcD-0006X6-1S for qemu-devel@nongnu.org; Wed, 26 Nov 2014 04:10:25 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAQ9AMW6013856 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 26 Nov 2014 04:10:22 -0500 Message-ID: <547598FC.6090001@redhat.com> Date: Wed, 26 Nov 2014 10:10:20 +0100 From: Max Reitz MIME-Version: 1.0 References: <1416822200-31088-1-git-send-email-mreitz@redhat.com> <1416822200-31088-3-git-send-email-mreitz@redhat.com> <5474DF13.4020202@redhat.com> In-Reply-To: <5474DF13.4020202@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/5] block: JSON filenames and relative backing files List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi On 2014-11-25 at 20:57, Eric Blake wrote: > On 11/24/2014 02:43 AM, Max Reitz wrote: >> When using a relative backing file name, qemu needs to know the >> directory of the top image file. For JSON filenames, such a directory >> cannot be easily determined (e.g. how do you determine the directory of >> a qcow2 BDS directly on top of a quorum BDS?). Therefore, do not allow >> relative filenames for the backing file of BDSs only having a JSON >> filename. >> >> Furthermore, BDS::exact_filename should be used whenever possible. If >> BDS::filename is not equal to BDS::exact_filename, the former will >> always be a JSON object. >> >> Signed-off-by: Max Reitz >> --- >> block.c | 27 +++++++++++++++++++++------ >> block/qapi.c | 7 ++++++- >> include/block/block.h | 5 +++-- >> 3 files changed, 30 insertions(+), 9 deletions(-) >> >> diff --git a/block.c b/block.c >> index 0c1be37..a0cddcd 100644 >> --- a/block.c >> +++ b/block.c >> @@ -305,19 +305,28 @@ void path_combine(char *dest, int dest_size, >> >> void bdrv_get_full_backing_filename_from_filename(const char *backed, >> const char *backing, >> - char *dest, size_t sz) >> + char *dest, size_t sz, >> + Error **errp) >> { >> - if (backing[0] == '\0' || path_has_protocol(backing)) { >> + if (backing[0] == '\0' || path_has_protocol(backing) || >> + path_is_absolute(backing)) >> + { > checkpatch.pl didn't complain about this? The { should be on the > previous line. With that fixed, > Reviewed-by: Eric Blake Oh, actually it does. But there's no rule about that in CODING_STYLE. There is a rule about { being on the same line as the if, though ("The opening brace is on the line that contains the control flow statement that introduces the new block"). With the condition split over multiple lines, that is no longer possible; therefore, we can place { anywhere we want. I always place { on the following line for multi-line conditions because I find that more readable[1]; maybe I'll change my mind in a year or two, but for now there is no rule about this case and I always do it this way; so far, nobody complained. :-) [1] I don't like the following: if (foo0() || foo1() || foo2() || foo3() || bar0() || bar1() || bar2() || bar3()) { baz(); Because in my eyes it looks like baz() may be part of the condition list at first glance (because it is indented just as much as the last line of the condition list). Max