On Mon, 8 Aug 2016 09:45:04 -0700 (PDT) no-re...@patchew.org wrote: <Trimmed the no-reply@ -- I wonder whether reply-to should be setup to something sensible, e.g. qemu-devel>
> 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.