Fix CASE invalid error expressions

[tcarmelveilleux]

Problem

• Some VerifyOrExit did not set err = properly in the
  macro, which dropped errors to the floor and raised
  warnings for some.

Change overview

• Added err = XXX where missing in VerifyOrExist calls

Testing

Testing done: ran all unit tests

[LuDuda]

Great catch!

I run the following regex: VerifyOrExit\((.*), CHIP_ERROR and see two other files "infected" (unless not done intentionally):

• https://github.com/project-chip/connectedhomeip/blob/master/src/controller/CHIPDeviceController.cpp#L915
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L624
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L790
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L817
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L1031

[tcarmelveilleux]

Great catch!

I run the following regex: VerifyOrExit\((.*), CHIP_ERROR and see two other files "infected" (unless not done intentionally):

• https://github.com/project-chip/connectedhomeip/blob/master/src/controller/CHIPDeviceController.cpp#L915
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L624
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L790
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L817
• https://github.com/project-chip/connectedhomeip/blob/master/src/platform/mbed/BLEManagerImpl.cpp#L1031

I fixed those too. Thanks for catching!

[bzbarsky-apple]

@tcarmelveilleux Needs rebase

[bzbarsky-apple]

I run the following regex: VerifyOrExit\((.*), CHIP_ERROR

How about we just add lints for this, since compiler warnings are not doing it? Something like so:

# Copyright (c) 2021 Project CHIP Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: Code lint

on:
    push:
    pull_request:
    workflow_dispatch:

jobs:
    code-lint:
        name: Codebase lints

        runs-on: ubuntu-18.04
        if: github.actor != 'restyled-io[bot]'

        steps:
            - name: Checkout
              uses: actions/checkout@v2
              with:
                  submodules: true

            # git grep exits with 0 if it finds a match, but we want
            # to fail (exit nonzero) on match.
            - name: Check for incorrect error use in VerifyOrExit
              run: |
                git grep -n "VerifyOrExit(.*, CHIP_ERROR" && exit 1 || exit 0

I can do a followup for that once this PR lands.

[Damian-Nordic]

Maybe we could use constexpr functions instead of constructors in the error macros to make [[nodiscard]] do the job?

struct ChipError
{
    [[nodiscard]]
    ChipError() = default;
};

[[nodiscard]]
constexpr ChipError MakeChipError() { return ChipError{}; }

#define CHIP_ERROR_DEFINED_OLD_WAY ChipError{}
#define CHIP_ERROR_DEFINED_NEW_WAY MakeChipError()

void foo()
{
    CHIP_ERROR_DEFINED_OLD_WAY; // does NOT generate warning
    CHIP_ERROR_DEFINED_NEW_WAY; // DOES generate warning
}

[bzbarsky-apple]

Maybe we could use constexpr functions instead of constructors in the error macros to make [[nodiscard]] do the job?

We could; we would need to change the existing places that do CHIP_ERROR(args) to make them benefit from this...

[github-actions]

Size increase report for "gn_qpg-example-build" from 40ac48e

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	-8	-8

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
[Unmapped],0,8
.debug_str,0,1
.debug_line,0,-2
.text,-8,-8
.debug_loc,0,-11

[github-actions]

Size increase report for "nrfconnect-example-build" from 40ac48e

File	Section	File	VM
chip-lock.elf	device_handles	4	4
chip-lock.elf	text	-4	-4

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
device_handles,4,4
.debug_line,0,-4
text,-4,-4
.debug_loc,0,-12

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize