From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43426) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bWnwN-0000tI-L1 for qemu-devel@nongnu.org; Mon, 08 Aug 2016 13:02:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bWnwJ-0006QM-Cs for qemu-devel@nongnu.org; Mon, 08 Aug 2016 13:02:14 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:28629 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bWnwJ-0006QG-6H for qemu-devel@nongnu.org; Mon, 08 Aug 2016 13:02:11 -0400 Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u78Gq5Iu043243 for ; Mon, 8 Aug 2016 13:02:10 -0400 Received: from e06smtp07.uk.ibm.com (e06smtp07.uk.ibm.com [195.75.94.103]) by mx0a-001b2d01.pphosted.com with ESMTP id 24nbs0h702-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 08 Aug 2016 13:02:10 -0400 Received: from localhost by e06smtp07.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 8 Aug 2016 18:02:08 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 5EA3017D8056 for ; Mon, 8 Aug 2016 18:03:44 +0100 (BST) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u78H25ER23003368 for ; Mon, 8 Aug 2016 17:02:05 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u78H255l024231 for ; Mon, 8 Aug 2016 11:02:05 -0600 Date: Mon, 8 Aug 2016 19:02:03 +0200 From: Cornelia Huck In-Reply-To: <201608081645.u78GKFHE092220@mx0b-001b2d01.pphosted.com> References: <1470670378-53732-1-git-send-email-dahi@linux.vnet.ibm.com> <201608081645.u78GKFHE092220@mx0b-001b2d01.pphosted.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20160808190203.2f8c8a60.cornelia.huck@de.ibm.com> Subject: Re: [Qemu-devel] [Patch v2 00/29] s390x CPU models: exposing features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: dahi@linux.vnet.ibm.com, famz@redhat.com, ehabkost@redhat.com, borntraeger@de.ibm.com, fiuczy@linux.vnet.ibm.com, imammedo@redhat.com, jdenemar@redhat.com, mimu@linux.vnet.ibm.com On Mon, 8 Aug 2016 09:45:04 -0700 (PDT) no-reply@patchew.org wrote: > Checking PATCH 4/29: s390x/cpumodel: introduce CPU features... > WARNING: line over 80 characters > #65: FILE: target-s390x/cpu_features.c:27: > + FEAT_INIT("zarch", S390_FEAT_TYPE_STFL, 1, "z/Architecture architectural mode"), We could conceivably break the line after one of the arguments, but there are some very long strings below. I'm not a fan of very long lines myself, but I wonder whether we should relax that limit? The Linux kernel has settled upon a relaxed limit and forbids splitting strings for greppability. > Checking PATCH 5/29: s390x/cpumodel: generate CPU feature lists for CPU models... > ERROR: Macros with complex values should be enclosed in parenthesis > #113: FILE: target-s390x/gen-features.c:20: > +#define S390_FEAT_GROUP_PLO \ > + S390_FEAT_PLO_CL, \ > + S390_FEAT_PLO_CLG, \ > + S390_FEAT_PLO_CLGR, \ > + S390_FEAT_PLO_CLX, \ > + S390_FEAT_PLO_CS, \ > + S390_FEAT_PLO_CSG, \ > + S390_FEAT_PLO_CSGR, \ > + S390_FEAT_PLO_CSX, \ > + S390_FEAT_PLO_DCS, \ > + S390_FEAT_PLO_DCSG, \ > + S390_FEAT_PLO_DCSGR, \ > + S390_FEAT_PLO_DCSX, \ > + S390_FEAT_PLO_CSST, \ > + S390_FEAT_PLO_CSSTG, \ > + S390_FEAT_PLO_CSSTGR, \ > + S390_FEAT_PLO_CSSTX, \ > + S390_FEAT_PLO_CSDST, \ > + S390_FEAT_PLO_CSDSTG, \ > + S390_FEAT_PLO_CSDSTGR, \ > + S390_FEAT_PLO_CSDSTX, \ > + S390_FEAT_PLO_CSTST, \ > + S390_FEAT_PLO_CSTSTG, \ > + S390_FEAT_PLO_CSTSTGR, \ > + S390_FEAT_PLO_CSTSTX I think the check is wrong to complain here. > Checking PATCH 6/29: s390x/cpumodel: generate CPU feature group lists... > Checking PATCH 7/29: s390x/cpumodel: introduce CPU feature group definitions... > WARNING: line over 80 characters > #71: FILE: target-s390x/cpu_features.c:363: > + FEAT_GROUP_INIT("gen13ptff", GEN13_PTFF, "PTFF enhancements introduced with z13"), Same comments as above. > Checking PATCH 8/29: s390x/cpumodel: register defined CPU models as subclasses... > WARNING: line over 80 characters > #64: FILE: target-s390x/cpu_models.c:30: > + .base_feat = { S390_FEAT_LIST_GEN ## _gen ## _GA ## _ec_ga ## _BASE }, \ And here. I honestly don't see a way to get this below 80 chars. > Checking PATCH 19/29: linux-headers: update against kvm/next... > ERROR: code indent should never use tabs > #21: FILE: include/standard-headers/linux/input-event-codes.h:615: > +#define KEY_RIGHT_UP^I^I^I0x266$ We should not check a linux headers update against qemu coding style. Either ignore the respective files, or check whether this patch is a linux headers update and nothing else. (I lack the perl skills for that :) > Checking PATCH 24/29: qmp: add QMP interface "query-cpu-model-expansion"... > WARNING: line over 80 characters > #29: FILE: include/sysemu/arch_init.h:38: > +CpuModelExpansionInfo *arch_query_cpu_model_expansion(CpuModelExpansionType type, This is a case of VeryLongVariableNames. Not sure whether we should do anything about that. > Checking PATCH 27/29: s390x/cpumodel: implement QMP interface "query-cpu-model-expansion"... > WARNING: line over 80 characters > #152: FILE: target-s390x/cpu_models.c:408: > + s390_feat_bitmap_to_ascii(model->features, qdict, qdict_add_enabled_feat); > > WARNING: line over 80 characters > #165: FILE: target-s390x/cpu_models.c:421: > +CpuModelExpansionInfo *arch_query_cpu_model_expansion(CpuModelExpansionType type, > > ERROR: space prohibited before that close parenthesis ')' > #181: FILE: target-s390x/cpu_models.c:437: > + } else if (type != CPU_MODEL_EXPANSION_TYPE_FULL ) { That's actually a warning I concur with :) I think we should at least come up with a check for a linux header update to avoid spamming the mailing list. And it would be a good idea to have some consensus about the 80 char limit as well.