Hi Luiz, Thanks, patch series v4 is sent to address lines over 80 columns. There's still one instance left though (in profiles/health/mcap.h) which is much a hassle to fix. The code around seems to encourage commenting over 80 lines and there is no good example on how we should handle the situation. I decided to leave that as is. Cheers, Archie On Sat, 4 Sept 2021 at 07:11, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Archie, Tedd, > > On Wed, Sep 1, 2021 at 9:51 PM <bluez.test.bot@xxxxxxxxx> wrote: > > > > This is automated email and please do not reply to this email! > > > > Dear submitter, > > > > Thank you for submitting the patches to the linux bluetooth mailing list. > > This is a CI test results with your patch series: > > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=540975 > > > > ---Test result--- > > > > Test Summary: > > CheckPatch FAIL 4.45 seconds > > GitLint FAIL 1.28 seconds > > Prep - Setup ELL PASS 39.55 seconds > > Build - Prep PASS 0.10 seconds > > Build - Configure PASS 6.94 seconds > > Build - Make PASS 171.01 seconds > > Make Check PASS 8.97 seconds > > Make Distcheck PASS 207.59 seconds > > Build w/ext ELL - Configure PASS 7.26 seconds > > Build w/ext ELL - Make PASS 160.39 seconds > > > > Details > > ############################## > > Test: CheckPatch - FAIL > > Desc: Run checkpatch.pl script with rule in .checkpatch.conf > > Output: > > lib: Inclusive language changes > > ERROR:SPACING: space required after that ',' (ctx:VxV) > > #146: FILE: lib/hci.c:739: > > + { "<AFH class. perip.>",LMP_AFH_CLS_PRP }, /* Bit 4 */ > > ^ > > > > WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed)) > > #296: FILE: lib/hci.h:427: > > +} __attribute__ ((packed)) central_link_key_cp; > > > > WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed)) > > #312: FILE: lib/hci.h:1568: > > +} __attribute__ ((packed)) le_read_accept_list_size_rp; > > > > WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed)) > > #325: FILE: lib/hci.h:1577: > > +} __attribute__ ((packed)) le_add_device_to_accept_list_cp; > > > > WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed)) > > #335: FILE: lib/hci.h:1584: > > +} __attribute__ ((packed)) le_remove_device_from_accept_list_cp; > > > > WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed)) > > #352: FILE: lib/hci.h:1812: > > +} __attribute__ ((packed)) evt_central_link_key_complete; > > @Tedd we should probably shut down warnings like above, since that > only apply to the kernel tree. > > > WARNING:LONG_LINE: line length of 81 exceeds 80 columns > > #378: FILE: lib/hci_lib.h:120: > > +int hci_le_add_accept_list(int dd, const bdaddr_t *bdaddr, uint8_t type, int to); > > > > - total: 1 errors, 6 warnings, 531 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] lib: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > monitor: Inclusive language changes > > WARNING:LONG_LINE_STRING: line length of 83 exceeds 80 columns > > #1147: FILE: monitor/packet.c:3681: > > + print_field("Peripheral Conn. Interval: 0x%4.4x - 0x%4.4x", > > > > ERROR:TRAILING_WHITESPACE: trailing whitespace > > #1388: FILE: monitor/packet.c:8075: > > +^Iprint_field("Central to Peripheral Maximum SDU: 0x%4.4x", $ > > > > WARNING:LONG_LINE: line length of 84 exceeds 80 columns > > #1538: FILE: monitor/packet.c:10261: > > + const struct bt_hci_evt_peripheral_broadcast_channel_map_change *evt = data; > > @Archie Pusaka you will need to fix these long lines going past 80 > columns, also maybe it would have been better to cut > peripheral_broadcast from the struct name above since channel_map > should be enough to distinct from other events. > > > - total: 1 errors, 2 warnings, 1635 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > NOTE: Whitespace errors detected. > > You may wish to use scripts/cleanpatch or scripts/cleanfile > > > > "[PATCH] monitor: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > emulator: Inclusive language changes > > WARNING:LONG_LINE_COMMENT: line length of 81 exceeds 80 columns > > #130: FILE: emulator/btdev.c:3609: > > + * • any advertising filter policy uses the Accept List and advertising > > > > WARNING:LONG_LINE_COMMENT: line length of 81 exceeds 80 columns > > #177: FILE: emulator/btdev.c:3646: > > + * • any advertising filter policy uses the Accept List and advertising > > > > WARNING:LONG_LINE_COMMENT: line length of 81 exceeds 80 columns > > #232: FILE: emulator/btdev.c:3697: > > + * • any advertising filter policy uses the Accept List and advertising > > > > WARNING:LONG_LINE_COMMENT: line length of 84 exceeds 80 columns > > #383: FILE: emulator/btdev.c:6188: > > + btdev->le_features[0] |= 0x08; /* Peripheral-initiated Features Exchange */ > > > > WARNING:LONG_LINE_COMMENT: line length of 84 exceeds 80 columns > > #607: FILE: emulator/le.c:346: > > + //hci->le_features[0] |= 0x08; /* Peripheral-initiated Features Exchange */ > > > > WARNING:LONG_LINE_COMMENT: line length of 84 exceeds 80 columns > > #628: FILE: emulator/le.c:402: > > + hci->le_states[0] |= 0x40; /* Initiating + Connection (Central Role) */ > > > > - total: 0 errors, 6 warnings, 894 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] emulator: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > tools: Inclusive language changes > > WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) > > #10: > > https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf > > > > WARNING:LONG_LINE: line length of 92 exceeds 80 columns > > #340: FILE: tools/hciconfig.c:1925: > > + { "block", cmd_block, "<bdaddr>", "Add a device to the reject list" }, > > > > WARNING:LONG_LINE: line length of 97 exceeds 80 columns > > #341: FILE: tools/hciconfig.c:1926: > > + { "unblock", cmd_unblock, "<bdaddr>", "Remove a device from the reject list" }, > > > > ERROR:INITIALISED_STATIC: do not initialise statics to 0 > > #861: FILE: tools/l2test.c:113: > > +static int central = 0; > > > > WARNING:MISSING_SPACE: break quoted strings at a space character > > #906: FILE: tools/l2test.c:1340: > > while ((opt = getopt(argc, argv, "a:b:cde:g:i:mnpqrstuwxyz" > > + "AB:CD:EF:GH:I:J:K:L:M:N:O:P:Q:RSTUV:W:X:Y:Z:")) != EOF) { > > > > ERROR:FUNCTION_WITHOUT_ARGS: Bad function definition - void setup_accept_list() should probably be void setup_accept_list(void) > > #1033: FILE: tools/mesh-gatt/net.c:944: > > +static void setup_accept_list() > > > > WARNING:LONG_LINE_STRING: line length of 86 exceeds 80 columns > > #1657: FILE: tools/mgmt-tester.c:11605: > > + test_bredrle50_full("Ext Adv. connectable & connected (peripheral) - Success", > > > > WARNING:LONG_LINE_STRING: line length of 90 exceeds 80 columns > > #1664: FILE: tools/mgmt-tester.c:11610: > > + test_bredrle50_full("Ext Adv. non-connectable & connected (peripheral) - Success", > > > > WARNING:LONG_LINE_STRING: line length of 83 exceeds 80 columns > > #1671: FILE: tools/mgmt-tester.c:11615: > > + test_bredrle50_full("Ext Adv. connectable & connected (central) - Success", > > > > WARNING:LONG_LINE_STRING: line length of 87 exceeds 80 columns > > #1678: FILE: tools/mgmt-tester.c:11620: > > + test_bredrle50_full("Ext Adv. non-connectable & connected (central) - Success", > > > > WARNING:LONG_LINE_STRING: line length of 86 exceeds 80 columns > > #1725: FILE: tools/parser/avdtp.c:662: > > + printf("AVDTP(c): ver %d %s%scc %d %spt %d seqn %d time %d ssrc %d\n", > > > > WARNING:LONG_LINE: line length of 81 exceeds 80 columns > > #1874: FILE: tools/parser/hci.c:1643: > > + (cp->filter == 0x01 ? "accept list only" : "reserved"))); > > > > ERROR:INITIALISED_STATIC: do not initialise statics to 0 > > #2196: FILE: tools/rctest.c:79: > > +static int central = 0; > > > > WARNING:LONG_LINE: line length of 90 exceeds 80 columns > > #2239: FILE: tools/rctest.c:700: > > + while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:O:N:CAESL:W:M:D:Y:T")) != EOF) { > > > > ERROR:SPACING: spaces required around that '=' (ctx:VxV) > > #2239: FILE: tools/rctest.c:700: > > + while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:O:N:CAESL:W:M:D:Y:T")) != EOF) { > > ^ > > > > ERROR:SPACING: space required after that ',' (ctx:VxV) > > #2239: FILE: tools/rctest.c:700: > > + while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:O:N:CAESL:W:M:D:Y:T")) != EOF) { > > ^ > > > > ERROR:SPACING: space required after that ',' (ctx:VxV) > > #2239: FILE: tools/rctest.c:700: > > + while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:O:N:CAESL:W:M:D:Y:T")) != EOF) { > > ^ > > > > ERROR:INITIALISED_STATIC: do not initialise statics to 0 > > #2347: FILE: tools/rfcomm.c:40: > > +static int central = 0; > > > > WARNING:LONG_LINE: line length of 91 exceeds 80 columns > > #2383: FILE: tools/rfcomm.c:683: > > + while ((opt = getopt_long(argc, argv, "+i:rahAESCL:", main_options, NULL)) != -1) { > > > > - total: 7 errors, 12 warnings, 2159 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] tools: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > profiles: Inclusive language changes > > WARNING:LONG_LINE_COMMENT: line length of 85 exceeds 80 columns > > #87: FILE: profiles/health/mcap.c:56: > > + guint remote_caps; /* CSP-Peripheral: remote central got caps */ > > > > WARNING:LONG_LINE_COMMENT: line length of 90 exceeds 80 columns > > #88: FILE: profiles/health/mcap.c:57: > > + guint rem_req_acc; /* CSP-Peripheral: accuracy required by central */ > > > > WARNING:LONG_LINE_COMMENT: line length of 81 exceeds 80 columns > > #94: FILE: profiles/health/mcap.c:63: > > + void *csp_priv_data; /* CSP-Central: In-flight request data */ > > > > WARNING:LONG_LINE_COMMENT: line length of 100 exceeds 80 columns > > #116: FILE: profiles/health/mcap.h:273: > > + mcap_info_ind_event_cb mcl_sync_infoind_cb; /* (CSP Central) Received info indication */ > > > > - total: 0 errors, 4 warnings, 87 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] profiles: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > src: Inclusive language changes > > WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) > > #10: > > https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf > > > > ERROR:INITIALISED_STATIC: do not initialise statics to NULL > > #36: FILE: src/adapter.c:115: > > +static struct mgmt *mgmt_primary = NULL; > > > > - total: 1 errors, 1 warnings, 478 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] src: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > mesh: Inclusive language changes > > WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) > > #7: > > https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf > > > > ERROR:INITIALISED_STATIC: do not initialise statics to 0 > > #28: FILE: mesh/net-keys.c:56: > > +static uint32_t last_flooding_id = 0; > > > > - total: 1 errors, 1 warnings, 146 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] mesh: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > unit/mesh: Inclusive language changes > > WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) > > #7: > > https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf > > > > - total: 0 errors, 1 warnings, 16 lines checked > > > > NOTE: For some of the reported defects, checkpatch may be able to > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > "[PATCH] unit/mesh: Inclusive language changes" has style problems, please review. > > > > NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO > > > > NOTE: If any of the errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > You should probably add a hook like the following > > cat .git/hooks/pre-commit > exec git diff --cached | <pathto:checkpatch.pl> -q --no-signoff > --ignore CAMELCASE,NEW_TYPEDEFS,INITIALISED_STATIC,GLOBAL_INITIALISERS,PREFER_PACKED,SPACING,FSF_MAILING_ADDRESS,TRAILING_STATEMENTS,RETURN_VOID,FILE_PATH_CHANGES,COMPLEX_MACRO,SPLIT_STRING,OPEN_BRACE,MISSING_SPACE,STATIC_CONST_CHAR_ARRAY,LINE_SPACING,SYMBOLIC_PERMS,SPDX_LICENSE_TAG,SUSPECT_CODE_INDENT,SSCANF_TO_KSTRTO,EXECUTE_PERMISSIONS > --show-types - > > I should probably add something like this to HACKING so I don't have > to remind contributors how to use checkpatch locally. > > > > > ############################## > > Test: GitLint - FAIL > > Desc: Run gitlint with rule in .gitlint > > Output: > > tools: Inclusive language changes > > 7: B1 Line exceeds max length (95>80): "https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf"; > > > > src: Inclusive language changes > > 7: B1 Line exceeds max length (95>80): "https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf"; > > > > mesh: Inclusive language changes > > 4: B1 Line exceeds max length (95>80): "https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf"; > > > > unit/mesh: Inclusive language changes > > 4: B1 Line exceeds max length (95>80): "https://specificationrefs.bluetooth.com/language-mapping/Appropriate_Language_Mapping_Table.pdf"; > > > > > > ############################## > > Test: Prep - Setup ELL - PASS > > Desc: Clone, build, and install ELL > > > > ############################## > > Test: Build - Prep - PASS > > Desc: Prepare environment for build > > > > ############################## > > Test: Build - Configure - PASS > > Desc: Configure the BlueZ source tree > > > > ############################## > > Test: Build - Make - PASS > > Desc: Build the BlueZ source tree > > > > ############################## > > Test: Make Check - PASS > > Desc: Run 'make check' > > > > ############################## > > Test: Make Distcheck - PASS > > Desc: Run distcheck to check the distribution > > > > ############################## > > Test: Build w/ext ELL - Configure - PASS > > Desc: Configure BlueZ source with '--enable-external-ell' configuration > > > > ############################## > > Test: Build w/ext ELL - Make - PASS > > Desc: Build BlueZ source with '--enable-external-ell' configuration > > > > > > > > --- > > Regards, > > Linux Bluetooth > > > > > -- > Luiz Augusto von Dentz
