[{"id":1817864,"web_url":"http://patchwork.ozlabs.org/comment/1817864/","msgid":"<87d13qo6c6.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-12-07T16:23:05","subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Marc-André Lureau writes:\n\n> The C standard has the initial value at 0 and the subsequent values\n> incremented by 1. No need to set this explicitely.\n>\n> This will prevent from artificial \"gaps\" when compiling out some enum\n> values and having unnecessarily large MAX values & enums arrays.\n>\n> Signed-off-by: Marc-André Lureau \n> ---\n> scripts/qapi.py | 7 ++-----\n> 1 file changed, 2 insertions(+), 5 deletions(-)\n>\n> diff --git a/scripts/qapi.py b/scripts/qapi.py\n> index 94b735d8d6..074ee221a1 100644\n> --- a/scripts/qapi.py\n> +++ b/scripts/qapi.py\n> @@ -1985,14 +1985,11 @@ typedef enum %(c_name)s {\n> ''',\n> c_name=c_name(name))\n> \n> - i = 0\n> for value in enum_values:\n> ret += mcgen('''\n> - %(c_enum)s = %(i)d,\n> + %(c_enum)s,\n> ''',\n> - c_enum=c_enum_const(name, value, prefix),\n> - i=i)\n> - i += 1\n> + c_enum=c_enum_const(name, value, prefix))\n> \n> ret += mcgen('''\n> } %(c_name)s;\n\nRecapitulate review of v2: this risks entertaining mishaps like\ncompiling this one\n\n typedef enum Color {\n COLOR_WHITE,\n#if defined(NEED_CPU_H)\n#if defined(TARGET_S390X)\n COLOR_BLUE,\n#endif /* defined(TARGET_S390X) */\n#endif /* defined(NEED_CPU_H) */\n COLOR_BLACK,\n } Color;\n\nin s390x-code (COLOR_BLACK = 2) and in target-independent code\n(COLOR_BLACK = 1), then linking the two together.\n\nSame issue for struct members and such (previous patch).\n\nWhat's our story on preventing disaster here?\n\nIn the long run, we want to split the generated code so that\ntarget-specific and target-independent code are separate, and each part\nis always compiled with consistent preprocessor symbols. But I'm afraid\nthat's not in the card right now.\n\nI therefore proposed the stupidest temporary stopgap that could possibly\nwork: apply conditionals *only* to qmp-introspect.c, leave everything\nunconditional elsewhere.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yt14z5Y1Dz9sP9\n\tfor ;\n\tFri, 8 Dec 2017 03:23:46 +1100 (AEDT)","from localhost ([::1]:33087 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eMyxb-0004Eq-4U\n\tfor incoming@patchwork.ozlabs.org; Thu, 07 Dec 2017 11:23:43 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:56930)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eMyxB-0004EX-Gf\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 11:23:18 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eMyx5-0007aO-KZ\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 11:23:17 -0500","from mx1.redhat.com ([209.132.183.28]:48938)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1eMyx5-0007a1-EH\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 11:23:11 -0500","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 4A807C0587CC;\n\tThu, 7 Dec 2017 16:23:10 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-74.ams2.redhat.com\n\t[10.36.116.74])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 5E76360BE5;\n\tThu, 7 Dec 2017 16:23:07 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid E183A1138658; Thu, 7 Dec 2017 17:23:05 +0100 (CET)"],"From":"Markus Armbruster ","To":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau ","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-18-marcandre.lureau@redhat.com>","Date":"Thu, 07 Dec 2017 17:23:05 +0100","In-Reply-To":"<20170911110623.24981-18-marcandre.lureau@redhat.com> (\n\t=?utf-8?b?Ik1hcmMtQW5kcsOp?= Lureau\"'s message of \"Mon,\n\t11 Sep 2017 \t13:05:50 +0200\")","Message-ID":"<87d13qo6c6.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tThu, 07 Dec 2017 16:23:10 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1817910,"web_url":"http://patchwork.ozlabs.org/comment/1817910/","msgid":"","list_archive_url":null,"date":"2017-12-07T17:01:29","subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","submitter":{"id":6442,"url":"http://patchwork.ozlabs.org/api/people/6442/","name":"Marc-André Lureau","email":"marcandre.lureau@gmail.com"},"content":"On Thu, Dec 7, 2017 at 5:23 PM, Markus Armbruster wrote:\n> Marc-André Lureau writes:\n>\n>> The C standard has the initial value at 0 and the subsequent values\n>> incremented by 1. No need to set this explicitely.\n>>\n>> This will prevent from artificial \"gaps\" when compiling out some enum\n>> values and having unnecessarily large MAX values & enums arrays.\n>>\n>> Signed-off-by: Marc-André Lureau \n>> ---\n>> scripts/qapi.py | 7 ++-----\n>> 1 file changed, 2 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/scripts/qapi.py b/scripts/qapi.py\n>> index 94b735d8d6..074ee221a1 100644\n>> --- a/scripts/qapi.py\n>> +++ b/scripts/qapi.py\n>> @@ -1985,14 +1985,11 @@ typedef enum %(c_name)s {\n>> ''',\n>> c_name=c_name(name))\n>>\n>> - i = 0\n>> for value in enum_values:\n>> ret += mcgen('''\n>> - %(c_enum)s = %(i)d,\n>> + %(c_enum)s,\n>> ''',\n>> - c_enum=c_enum_const(name, value, prefix),\n>> - i=i)\n>> - i += 1\n>> + c_enum=c_enum_const(name, value, prefix))\n>>\n>> ret += mcgen('''\n>> } %(c_name)s;\n>\n> Recapitulate review of v2: this risks entertaining mishaps like\n> compiling this one\n>\n> typedef enum Color {\n> COLOR_WHITE,\n> #if defined(NEED_CPU_H)\n> #if defined(TARGET_S390X)\n> COLOR_BLUE,\n> #endif /* defined(TARGET_S390X) */\n> #endif /* defined(NEED_CPU_H) */\n> COLOR_BLACK,\n> } Color;\n>\n> in s390x-code (COLOR_BLACK = 2) and in target-independent code\n> (COLOR_BLACK = 1), then linking the two together.\n>\n> Same issue for struct members and such (previous patch).\n>\n> What's our story on preventing disaster here?\n>\n> In the long run, we want to split the generated code so that\n> target-specific and target-independent code are separate, and each part\n> is always compiled with consistent preprocessor symbols. But I'm afraid\n> that's not in the card right now.\n\nEh, I need to refresh my memories about that series, but I think\nthat's what I did in v3\n\nIt doesn't use the NEED_CPU_H trick. It has a seperate per-target target.json\n\n\n>\n> I therefore proposed the stupidest temporary stopgap that could possibly\n> work: apply conditionals *only* to qmp-introspect.c, leave everything\n> unconditional elsewhere.\n\nI don't like that idea much and I don't think we need that\nrestriction, but I need to get back to that series on some point\n(probably after you finish the review).","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"OokRJtQy\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yt2C05t3rz9sRW\n\tfor ;\n\tFri, 8 Dec 2017 04:14:04 +1100 (AEDT)","from localhost ([::1]:33484 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eMzkI-0008Ho-T8\n\tfor incoming@patchwork.ozlabs.org; Thu, 07 Dec 2017 12:14:02 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:42290)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eMzYC-00036P-V5\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 12:01:34 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eMzYC-0003NR-1I\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 12:01:33 -0500","from mail-wr0-x22e.google.com ([2a00:1450:400c:c0c::22e]:38151)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1eMzYB-0003N9-Pr\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 12:01:31 -0500","by mail-wr0-x22e.google.com with SMTP id o2so8221509wro.5\n\tfor ; Thu, 07 Dec 2017 09:01:31 -0800 (PST)","by 10.223.165.153 with HTTP; Thu, 7 Dec 2017 09:01:29 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=KCylgXie1NRuQT/u4YoEIbiUle0TlkBykvdVN8gZlBE=;\n\tb=OokRJtQyQau6iYH8wY7MZHQ5BilS80N1ZZE9PMDukY/2Ak3nE/c55X9qXsTKRPOEkA\n\tjJYQzeWStmWa40sl1DF+sX5hMLjgc86Ds5kr0rczwDMyzqEu1dmcQkduCqPSIZoQOFkj\n\t+B6pXmmZCmlXVxgKRmg+i1jsIWYvvbNs3a69/Gc62BvzbIaWSn5XCaJdqOp14t5sthjP\n\tE0rvu6mlBzXUkWFK1OL5X9ZzQF6hvY2aZVqPJOj86jaRDM6QA4oC1eJ3YXgxQyKc9mYB\n\tNUDUCtXqYHmAAceBwND1vKkuJzhSeOt8utRZmdz/FIE6gxKsQTQndYQcbyLok4RKfYJo\n\tHF7Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=KCylgXie1NRuQT/u4YoEIbiUle0TlkBykvdVN8gZlBE=;\n\tb=UvD1lQZJGOIVpkueZz3zITWYPQrpi0T/jlJq5xb1k9/lX/aURVnZG3Rs04kww5/1HP\n\tlB86z+4Ms9u6vXhomkjWDH2k5sJVQVkUzTX7OaMiOwYkf73fI+8X/9kukotyRzKLPTnZ\n\tVhZTXRZm8a39jGCSuaKtSEd654WRXlfZcnvZe6BXvW+Dddj7hDPlU3iDSe8kEj46WCPb\n\tTZyiTT8GPfljMDxH5C4xRZ2LaoOggGki1mZyBmbcru8RsrcOlNC5VlG2PrkNFTsaTIsI\n\ts7JaWZ23wJj+08aDH1GT8yHe5YtuZnhF/g15Dtbu9kgNSCDcEmwq9J/23stRK9U3L+ug\n\thhRQ==","X-Gm-Message-State":"AKGB3mIbC9hjpUxFxpWIQGk/mgW0czsD7Xj6ElZDZp/9O91Z/LRajyJW\n\tdy2FoOdv4OyqjF1lqFlpwBEi0XiNpas/TKwI8go=","X-Google-Smtp-Source":"AGs4zMZ66L/Pve/YsOimXW9IAD9AH+dClRX5/3O6lusbRy+GUnTTizt5s2vYz3B0gB2PQx6WV+KMOhuHAjWnOIafq0M=","X-Received":"by 10.223.199.203 with SMTP id y11mr18873970wrg.37.1512666090188;\n\tThu, 07 Dec 2017 09:01:30 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<87d13qo6c6.fsf@dusky.pond.sub.org>","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-18-marcandre.lureau@redhat.com>\n\t<87d13qo6c6.fsf@dusky.pond.sub.org>","From":"=?utf-8?q?Marc-Andr=C3=A9_Lureau?= ","Date":"Thu, 7 Dec 2017 18:01:29 +0100","Message-ID":"","To":"Markus Armbruster ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c0c::22e","Subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"QEMU , Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1818382,"web_url":"http://patchwork.ozlabs.org/comment/1818382/","msgid":"<87zi6tirpa.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-12-08T07:50:25","subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Marc-André Lureau writes:\n\n> On Thu, Dec 7, 2017 at 5:23 PM, Markus Armbruster wrote:\n>> Marc-André Lureau writes:\n>>\n>>> The C standard has the initial value at 0 and the subsequent values\n>>> incremented by 1. No need to set this explicitely.\n>>>\n>>> This will prevent from artificial \"gaps\" when compiling out some enum\n>>> values and having unnecessarily large MAX values & enums arrays.\n>>>\n>>> Signed-off-by: Marc-André Lureau \n>>> ---\n>>> scripts/qapi.py | 7 ++-----\n>>> 1 file changed, 2 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/scripts/qapi.py b/scripts/qapi.py\n>>> index 94b735d8d6..074ee221a1 100644\n>>> --- a/scripts/qapi.py\n>>> +++ b/scripts/qapi.py\n>>> @@ -1985,14 +1985,11 @@ typedef enum %(c_name)s {\n>>> ''',\n>>> c_name=c_name(name))\n>>>\n>>> - i = 0\n>>> for value in enum_values:\n>>> ret += mcgen('''\n>>> - %(c_enum)s = %(i)d,\n>>> + %(c_enum)s,\n>>> ''',\n>>> - c_enum=c_enum_const(name, value, prefix),\n>>> - i=i)\n>>> - i += 1\n>>> + c_enum=c_enum_const(name, value, prefix))\n>>>\n>>> ret += mcgen('''\n>>> } %(c_name)s;\n>>\n>> Recapitulate review of v2: this risks entertaining mishaps like\n>> compiling this one\n>>\n>> typedef enum Color {\n>> COLOR_WHITE,\n>> #if defined(NEED_CPU_H)\n>> #if defined(TARGET_S390X)\n>> COLOR_BLUE,\n>> #endif /* defined(TARGET_S390X) */\n>> #endif /* defined(NEED_CPU_H) */\n>> COLOR_BLACK,\n>> } Color;\n>>\n>> in s390x-code (COLOR_BLACK = 2) and in target-independent code\n>> (COLOR_BLACK = 1), then linking the two together.\n>>\n>> Same issue for struct members and such (previous patch).\n>>\n>> What's our story on preventing disaster here?\n>>\n>> In the long run, we want to split the generated code so that\n>> target-specific and target-independent code are separate, and each part\n>> is always compiled with consistent preprocessor symbols. But I'm afraid\n>> that's not in the card right now.\n>\n> Eh, I need to refresh my memories about that series, but I think\n> that's what I did in v3\n>\n> It doesn't use the NEED_CPU_H trick. It has a seperate per-target target.json\n\nLooking... aha! target.json appears in PATCH 44 (which I haven't even\nglanced at, yet). The problem appears in PATCH 16, though. Perhaps a\nbit of patch reshuffling would do.\n\n>> I therefore proposed the stupidest temporary stopgap that could possibly\n>> work: apply conditionals *only* to qmp-introspect.c, leave everything\n>> unconditional elsewhere.\n>\n> I don't like that idea much and I don't think we need that\n> restriction, but I need to get back to that series on some point\n> (probably after you finish the review).\n\nIt's a beefy series, and it's probably best to let me review the largest\nprefix I can before we dive into discussion.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3ytPfn5jRQz9s71\n\tfor ;\n\tFri, 8 Dec 2017 18:50:57 +1100 (AEDT)","from localhost ([::1]:35915 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eNDQt-0007f4-VZ\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Dec 2017 02:50:55 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:59907)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eNDQW-0007dk-AD\n\tfor qemu-devel@nongnu.org; Fri, 08 Dec 2017 02:50:34 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eNDQS-0001wG-VL\n\tfor qemu-devel@nongnu.org; Fri, 08 Dec 2017 02:50:32 -0500","from mx1.redhat.com ([209.132.183.28]:57740)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1eNDQS-0001v1-Mf\n\tfor qemu-devel@nongnu.org; Fri, 08 Dec 2017 02:50:28 -0500","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id C83735D5F4;\n\tFri, 8 Dec 2017 07:50:27 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-74.ams2.redhat.com\n\t[10.36.116.74])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 6DC109230A;\n\tFri, 8 Dec 2017 07:50:27 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid F3F2C1138658; Fri, 8 Dec 2017 08:50:25 +0100 (CET)"],"From":"Markus Armbruster ","To":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau ","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-18-marcandre.lureau@redhat.com>\n\t<87d13qo6c6.fsf@dusky.pond.sub.org>\n\t","Date":"Fri, 08 Dec 2017 08:50:25 +0100","In-Reply-To":"\n\t( =?utf-8?b?Ik1hcmMtQW5kcsOp?= Lureau\"'s message of \"Thu,\n\t7 Dec 2017 \t18:01:29 +0100\")","Message-ID":"<87zi6tirpa.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tFri, 08 Dec 2017 07:50:27 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"QEMU , Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1837334,"web_url":"http://patchwork.ozlabs.org/comment/1837334/","msgid":"","list_archive_url":null,"date":"2018-01-11T21:24:03","subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","submitter":{"id":6442,"url":"http://patchwork.ozlabs.org/api/people/6442/","name":"Marc-André Lureau","email":"marcandre.lureau@gmail.com"},"content":"Hi\n\nOn Fri, Dec 8, 2017 at 8:50 AM, Markus Armbruster wrote:\n> Marc-André Lureau writes:\n>\n>> On Thu, Dec 7, 2017 at 5:23 PM, Markus Armbruster wrote:\n>>> Marc-André Lureau writes:\n>>>\n>>>> The C standard has the initial value at 0 and the subsequent values\n>>>> incremented by 1. No need to set this explicitely.\n>>>>\n>>>> This will prevent from artificial \"gaps\" when compiling out some enum\n>>>> values and having unnecessarily large MAX values & enums arrays.\n>>>>\n>>>> Signed-off-by: Marc-André Lureau \n>>>> ---\n>>>> scripts/qapi.py | 7 ++-----\n>>>> 1 file changed, 2 insertions(+), 5 deletions(-)\n>>>>\n>>>> diff --git a/scripts/qapi.py b/scripts/qapi.py\n>>>> index 94b735d8d6..074ee221a1 100644\n>>>> --- a/scripts/qapi.py\n>>>> +++ b/scripts/qapi.py\n>>>> @@ -1985,14 +1985,11 @@ typedef enum %(c_name)s {\n>>>> ''',\n>>>> c_name=c_name(name))\n>>>>\n>>>> - i = 0\n>>>> for value in enum_values:\n>>>> ret += mcgen('''\n>>>> - %(c_enum)s = %(i)d,\n>>>> + %(c_enum)s,\n>>>> ''',\n>>>> - c_enum=c_enum_const(name, value, prefix),\n>>>> - i=i)\n>>>> - i += 1\n>>>> + c_enum=c_enum_const(name, value, prefix))\n>>>>\n>>>> ret += mcgen('''\n>>>> } %(c_name)s;\n>>>\n>>> Recapitulate review of v2: this risks entertaining mishaps like\n>>> compiling this one\n>>>\n>>> typedef enum Color {\n>>> COLOR_WHITE,\n>>> #if defined(NEED_CPU_H)\n>>> #if defined(TARGET_S390X)\n>>> COLOR_BLUE,\n>>> #endif /* defined(TARGET_S390X) */\n>>> #endif /* defined(NEED_CPU_H) */\n>>> COLOR_BLACK,\n>>> } Color;\n>>>\n>>> in s390x-code (COLOR_BLACK = 2) and in target-independent code\n>>> (COLOR_BLACK = 1), then linking the two together.\n>>>\n>>> Same issue for struct members and such (previous patch).\n>>>\n>>> What's our story on preventing disaster here?\n>>>\n>>> In the long run, we want to split the generated code so that\n>>> target-specific and target-independent code are separate, and each part\n>>> is always compiled with consistent preprocessor symbols. But I'm afraid\n>>> that's not in the card right now.\n>>\n>> Eh, I need to refresh my memories about that series, but I think\n>> that's what I did in v3\n>>\n>> It doesn't use the NEED_CPU_H trick. It has a seperate per-target target.json\n>\n> Looking... aha! target.json appears in PATCH 44 (which I haven't even\n> glanced at, yet). The problem appears in PATCH 16, though. Perhaps a\n> bit of patch reshuffling would do.\n\nWhat problem appears in patch 16? Some code could be introduced using\nNEED_CPU_H and link arch & independent code together? It is still true\nafter patch 44. If necessary, I can work on a split-qapi series before\nthe conditionals are added. But the real benefit is only apparent\nafter the conditional are introduced, so I am not motivated to\nreorder.\n\n>\n>>> I therefore proposed the stupidest temporary stopgap that could possibly\n>>> work: apply conditionals *only* to qmp-introspect.c, leave everything\n>>> unconditional elsewhere.\n>>\n>> I don't like that idea much and I don't think we need that\n>> restriction, but I need to get back to that series on some point\n>> (probably after you finish the review).\n>\n> It's a beefy series, and it's probably best to let me review the largest\n> prefix I can before we dive into discussion.\n\nthanks","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"gMKsOu+Y\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3zHfF10tf4z9sPk\n\tfor ;\n\tFri, 12 Jan 2018 08:30:45 +1100 (AEDT)","from localhost ([::1]:44387 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eZkQt-0001X7-6S\n\tfor incoming@patchwork.ozlabs.org; Thu, 11 Jan 2018 16:30:43 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:44597)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eZkKU-0004OX-78\n\tfor qemu-devel@nongnu.org; Thu, 11 Jan 2018 16:24:07 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eZkKT-0000w4-5n\n\tfor qemu-devel@nongnu.org; Thu, 11 Jan 2018 16:24:06 -0500","from mail-wr0-x231.google.com ([2a00:1450:400c:c0c::231]:37551)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1eZkKS-0000vN-Vh\n\tfor qemu-devel@nongnu.org; Thu, 11 Jan 2018 16:24:05 -0500","by mail-wr0-x231.google.com with SMTP id f8so3542179wre.4\n\tfor ; Thu, 11 Jan 2018 13:24:04 -0800 (PST)","by 10.223.199.143 with HTTP; Thu, 11 Jan 2018 13:24:03 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=3RVXEoKQQ0J1L9hMxFRf5YnPqMkAHWgKFAXKpGp4JnM=;\n\tb=gMKsOu+YXe3wj0bkxY4VD0Ry2cH+dw3ylWpy+dQ2o1DiBFmzVZiqnx1COOE5INW2Ou\n\tGNnplpsdzNnwI7yS+euT0yxYlIu+qLkSan7fp/EN/oUIJpPH0md0gQk7kz88lwozUOB1\n\tww6mP9ux44i6x7jgiTPc2a8BXS9TNdoKyJdhiB3FyAqhgtVikLdmDh0YftVg1dKTXb3l\n\tAlPSSFBL4zLD8kfTCbwRAHzjNcpulaFN5qSzqpPDA1pznxXNaYozLsZ4efeG2F7+F0Pm\n\t/UE+FEx98xHt7ijjtzwq+lIqe/dSuwCpk67WMvO8cyLPQzMQgUDUcYH5NBAPbjUyMqFl\n\t5X+A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=3RVXEoKQQ0J1L9hMxFRf5YnPqMkAHWgKFAXKpGp4JnM=;\n\tb=RVUL+wEWkrFOhwoUk3YfByj68MHLC33kQ97yK01nL0MkYWAO9IpZ2y2LozVyXd1v/g\n\tjSIUCroaaKTG9HDenR3+fuinNGoyLVaufqWyQBUFZW1XMcRibYMDXWerSZxW4kauqE4C\n\tF10mM5qGHMFwwnnGfRnZfCkDdQPHEhbiw7G9A6LHsLViqdU+R/Yoi/R3ikSZMdMUF1WW\n\tqfR7gvj4lo/h5GI19cRgYbVXf5p7qCL9QzyhU1e3FgIkR/s7TQO0AEqO6MuLNiYRCYlb\n\tQq08gYpeoN/MMtRjy/w8WOd3jkFsBFFWBR6auN7F7wdD50j8HrRENaz2urN5WZ5P2YGQ\n\t/5Pw==","X-Gm-Message-State":"AKGB3mJxSnEoj0RVUgoYCMPGJSpO1HAPeFswLB1C9A8HYqodiND6ZDwq\n\tf87+c6MfUlQ+hyrcRo7n00KYXftc/D/CG2M63Ko=","X-Google-Smtp-Source":"ACJfBosqdVQ2ga8Hbbvp8ZgElkPoJCpypqox/6zemRyvbMHzdq7VYlyscWY8Ehkx7TF9xpInC+Y+g35hqE8qusH3AIs=","X-Received":"by 10.223.197.69 with SMTP id s5mr22654242wrf.96.1515705843960; \n\tThu, 11 Jan 2018 13:24:03 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<87zi6tirpa.fsf@dusky.pond.sub.org>","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-18-marcandre.lureau@redhat.com>\n\t<87d13qo6c6.fsf@dusky.pond.sub.org>\n\t\n\t<87zi6tirpa.fsf@dusky.pond.sub.org>","From":"=?utf-8?q?Marc-Andr=C3=A9_Lureau?= ","Date":"Thu, 11 Jan 2018 22:24:03 +0100","Message-ID":"","To":"Markus Armbruster ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c0c::231","Subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"QEMU , Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1850524,"web_url":"http://patchwork.ozlabs.org/comment/1850524/","msgid":"<87a7wrlaev.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2018-02-02T14:43:20","subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Marc-André Lureau writes:\n\n> Hi\n>\n> On Fri, Dec 8, 2017 at 8:50 AM, Markus Armbruster wrote:\n>> Marc-André Lureau writes:\n>>\n>>> On Thu, Dec 7, 2017 at 5:23 PM, Markus Armbruster wrote:\n>>>> Marc-André Lureau writes:\n>>>>\n>>>>> The C standard has the initial value at 0 and the subsequent values\n>>>>> incremented by 1. No need to set this explicitely.\n>>>>>\n>>>>> This will prevent from artificial \"gaps\" when compiling out some enum\n>>>>> values and having unnecessarily large MAX values & enums arrays.\n>>>>>\n>>>>> Signed-off-by: Marc-André Lureau \n>>>>> ---\n>>>>> scripts/qapi.py | 7 ++-----\n>>>>> 1 file changed, 2 insertions(+), 5 deletions(-)\n>>>>>\n>>>>> diff --git a/scripts/qapi.py b/scripts/qapi.py\n>>>>> index 94b735d8d6..074ee221a1 100644\n>>>>> --- a/scripts/qapi.py\n>>>>> +++ b/scripts/qapi.py\n>>>>> @@ -1985,14 +1985,11 @@ typedef enum %(c_name)s {\n>>>>> ''',\n>>>>> c_name=c_name(name))\n>>>>>\n>>>>> - i = 0\n>>>>> for value in enum_values:\n>>>>> ret += mcgen('''\n>>>>> - %(c_enum)s = %(i)d,\n>>>>> + %(c_enum)s,\n>>>>> ''',\n>>>>> - c_enum=c_enum_const(name, value, prefix),\n>>>>> - i=i)\n>>>>> - i += 1\n>>>>> + c_enum=c_enum_const(name, value, prefix))\n>>>>>\n>>>>> ret += mcgen('''\n>>>>> } %(c_name)s;\n>>>>\n>>>> Recapitulate review of v2: this risks entertaining mishaps like\n>>>> compiling this one\n>>>>\n>>>> typedef enum Color {\n>>>> COLOR_WHITE,\n>>>> #if defined(NEED_CPU_H)\n>>>> #if defined(TARGET_S390X)\n>>>> COLOR_BLUE,\n>>>> #endif /* defined(TARGET_S390X) */\n>>>> #endif /* defined(NEED_CPU_H) */\n>>>> COLOR_BLACK,\n>>>> } Color;\n>>>>\n>>>> in s390x-code (COLOR_BLACK = 2) and in target-independent code\n>>>> (COLOR_BLACK = 1), then linking the two together.\n>>>>\n>>>> Same issue for struct members and such (previous patch).\n>>>>\n>>>> What's our story on preventing disaster here?\n>>>>\n>>>> In the long run, we want to split the generated code so that\n>>>> target-specific and target-independent code are separate, and each part\n>>>> is always compiled with consistent preprocessor symbols. But I'm afraid\n>>>> that's not in the card right now.\n>>>\n>>> Eh, I need to refresh my memories about that series, but I think\n>>> that's what I did in v3\n>>>\n>>> It doesn't use the NEED_CPU_H trick. It has a seperate per-target target.json\n>>\n>> Looking... aha! target.json appears in PATCH 44 (which I haven't even\n>> glanced at, yet). The problem appears in PATCH 16, though. Perhaps a\n>> bit of patch reshuffling would do.\n>\n> What problem appears in patch 16? Some code could be introduced using\n> NEED_CPU_H and link arch & independent code together?\n\nIt's been a while...\n\nGenerated headers using conditionals must include the headers providing\nthe symbols used in conditionals. Not doing so is an open death trap.\n\nPATCH 16 sets up the first instance of the death trap. Or maybe it's\nPATCH 13.\n\nHowever, including these headers only becomes possible *after* you split\noff the target-specific stuff in PATCH 44.\n\nDo I make any sense?\n\n> It is still true\n> after patch 44. If necessary, I can work on a split-qapi series before\n> the conditionals are added. But the real benefit is only apparent\n> after the conditional are introduced, so I am not motivated to\n> reorder.\n\nUnderstand.\n\nAs a maintainer, I can ask for improvements, but the only lever I have\nis saying no. Which should be reserved for cases that are actually\nwrong, or create inacceptable technical debt. Temporary death traps\ndon't count as either. For cases I merely hate, when asking doesn't\nhelp, all I can do is do the work myself. So I did:\n\n [PATCH RFC 00/21] Modularize generated QAPI code\n Message-Id: <20180202130336.24719-1-armbru@redhat.com>\n\n[...]","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3zY0jq4d4rz9t0m\n\tfor ;\n\tSat, 3 Feb 2018 02:08:30 +1100 (AEDT)","from localhost ([::1]:36054 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1ehcx2-0004J7-7Y\n\tfor incoming@patchwork.ozlabs.org; Fri, 02 Feb 2018 10:08:28 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:38141)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1ehcYq-0000CV-JA\n\tfor qemu-devel@nongnu.org; Fri, 02 Feb 2018 09:43:29 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1ehcYl-0001vI-OF\n\tfor qemu-devel@nongnu.org; Fri, 02 Feb 2018 09:43:28 -0500","from mx1.redhat.com ([209.132.183.28]:11663)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1ehcYl-0001uh-Fx\n\tfor qemu-devel@nongnu.org; Fri, 02 Feb 2018 09:43:23 -0500","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 60315C036741;\n\tFri, 2 Feb 2018 14:43:22 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-148.ams2.redhat.com\n\t[10.36.116.148])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 094827A25F;\n\tFri, 2 Feb 2018 14:43:22 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 8D5A01138645; Fri, 2 Feb 2018 15:43:20 +0100 (CET)"],"From":"Markus Armbruster ","To":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau ","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-18-marcandre.lureau@redhat.com>\n\t<87d13qo6c6.fsf@dusky.pond.sub.org>\n\t\n\t<87zi6tirpa.fsf@dusky.pond.sub.org>\n\t","Date":"Fri, 02 Feb 2018 15:43:20 +0100","In-Reply-To":"\n\t( =?utf-8?b?Ik1hcmMtQW5kcsOp?= Lureau\"'s message of \"Thu,\n\t11 Jan 2018 \t22:24:03 +0100\")","Message-ID":"<87a7wrlaev.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tFri, 02 Feb 2018 14:43:22 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 17/50] qapi: do not define enumeration\n\tvalue explicitely","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"QEMU , Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]