Hi Igor,

thank you for the patch. Yeah I was not quick enough to reply on the PR :)

Acked-by: Ales Musil <amu...@redhat.com>


On Mon, Jul 11, 2022 at 1:10 PM Igor Zhukov <fsb4...@yandex.ru> wrote:

> From: Igor Zhukov <ivzhu...@sbercloud.ru>
>
> Also fix typo in comments
>
> Also fix memory leak in utilities/ovn-dbctl.c
>
> Also add "-g" to CFLAGS to forbid autotools from adding "-g -O2"
> (temporary solution)
>
> Signed-off-by: Igor Zhukov <ivzhu...@sbercloud.ru>
> Acked-by: Dumitru Ceara <dce...@redhat.com>
> Submitted-at: https://github.com/ovn-org/ovn/pull/139
> ---
>  .ci/linux-build.sh         | 16 +++++-----------
>  .github/workflows/test.yml | 10 ++++------
>  tests/atlocal.in           |  4 ++--
>  tests/automake.mk          |  3 +--
>  tests/ovs-macros.at        | 11 +++--------
>  utilities/ovn-dbctl.c      |  4 ++++
>  6 files changed, 19 insertions(+), 29 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index 570ddbace..8c6d751dc 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -28,20 +28,14 @@ function configure_ovn()
>  save_OPTS="${OPTS} $*"
>  OPTS="${EXTRA_OPTS} ${save_OPTS}"
>
> -# If AddressSanitizer or UdefinedBehaviorSanitizer is requested, enable
> it,
> +# If AddressSanitizer and UndefinedBehaviorSanitizer are requested,
> enable them,
>  # but only for OVN, not for OVS.  However, disable some optimizations for
>  # OVS, to make sanitizer reports user friendly.
> -if [ "$ASAN" ]; then
> -    CFLAGS="-O1 -fno-omit-frame-pointer -fno-common"
> -    CFLAGS_ASAN="-fsanitize=address"
> -    OVN_CFLAGS="${OVN_CFLAGS} ${CFLAGS_ASAN}"
> -fi
> -
> -if [ "$UBSAN" ]; then
> +if [ "$SANITIZERS" ]; then
>      # Use the default options configured in tests/atlocal.in, in
> UBSAN_OPTIONS.
> -    CFLAGS="-O1 -fno-omit-frame-pointer -fno-common"
> -    CFLAGS_UBSAN="-fsanitize=undefined"
> -    OVN_CFLAGS="${OVN_CFLAGS} ${CFLAGS_UBSAN}"
> +    CFLAGS="-O1 -fno-omit-frame-pointer -fno-common -g"
> +    CFLAGS_SANITIZERS="-fsanitize=address,undefined"
> +    OVN_CFLAGS="${OVN_CFLAGS} ${CFLAGS_SANITIZERS}"
>  fi
>
>  if [ "$CC" = "clang" ]; then
> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> index b24bc7a3b..e0de7c60e 100644
> --- a/.github/workflows/test.yml
> +++ b/.github/workflows/test.yml
> @@ -20,8 +20,8 @@ jobs:
>        M32:         ${{ matrix.m32 }}
>        OPTS:        ${{ matrix.opts }}
>        TESTSUITE:   ${{ matrix.testsuite }}
> -      ASAN:        ${{ matrix.asan }}
> -      UBSAN:       ${{ matrix.ubsan }}
> +      SANITIZERS:  ${{ matrix.sanitizers }}
> +      CFLAGS:      ${{ matrix.cflags }}
>
>      name: linux ${{ join(matrix.*, ' ') }}
>      runs-on: ubuntu-20.04
> @@ -41,10 +41,8 @@ jobs:
>              testsuite:    system-test
>            - compiler:     clang
>              testsuite:    test
> -            asan:         asan
> -          - compiler:     clang
> -            testsuite:    test
> -            ubsan:        ubsan
> +            sanitizers:   sanitizers
> +            cflags:       -g
>
>            - compiler:     gcc
>              testsuite:    test
> diff --git a/tests/atlocal.in b/tests/atlocal.in
> index b3a011f41..0b9a31276 100644
> --- a/tests/atlocal.in
> +++ b/tests/atlocal.in
> @@ -211,10 +211,10 @@ export OVS_CTL_TIMEOUT
>
>  # Add some default flags to make the tests run better under Address
>  # Sanitizer, if it was used for the build.
>
> -ASAN_OPTIONS=detect_leaks=1:abort_on_error=true:log_path=asan:$ASAN_OPTIONS
>
> +ASAN_OPTIONS=detect_leaks=1:abort_on_error=true:log_path=sanitizers:$ASAN_OPTIONS
>  export ASAN_OPTIONS
>
>  # Add some default flags for UndefinedBehaviorSanitizer, if it was used
>  # for the build.
>
> -UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=true:log_path=ubsan:$UBSAN_OPTIONS
>
> +UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=true:log_path=sanitizers:$UBSAN_OPTIONS
>  export UBSAN_OPTIONS
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 9733e8fa0..dce9c9108 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -80,8 +80,7 @@ export ovs_srcdir
>  check-local:
>         set $(SHELL) '$(TESTSUITE)' -C tests
> AUTOTEST_PATH=$(AUTOTEST_PATH); \
>         "$$@" $(TESTSUITEFLAGS) || \
> -       (test -z "$$(find $(TESTSUITE_DIR) -name 'asan.*')" && \
> -        test -z "$$(find $(TESTSUITE_DIR) -name 'ubsan.*')" && \
> +       (test -z "$$(find $(TESTSUITE_DIR) -name 'sanitizers.*')" && \
>          test X'$(RECHECK)' = Xyes && "$$@" --recheck)
>
>  # Python Coverage support.
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 1bb31f315..5a06fe956 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -224,14 +224,9 @@ m4_divert_pop([PREPARE_TESTS])
>
>  OVS_START_SHELL_HELPERS
>  ovs_cleanup() {
> -    if test "$(echo asan.*)" != 'asan.*'; then
> -        echo "Address Sanitizer reported errors in:" asan.*
> -        cat asan.*
> -        AT_FAIL_IF([:])
> -    fi
> -    if test "$(echo ubsan.*)" != 'ubsan.*'; then
> -        echo "Undefined Behavior Sanitizer reported errors in:" ubsan.*
> -        cat ubsan.*
> +    if test "$(echo sanitizers.*)" != 'sanitizers.*'; then
> +        echo "Undefined Behavior Sanitizer or Address Sanitizer reported
> errors in:" sanitizers.*
> +        cat sanitizers.*
>          AT_FAIL_IF([:])
>      fi
>  }
> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
> index c4cc8c9b2..7ee0b92b4 100644
> --- a/utilities/ovn-dbctl.c
> +++ b/utilities/ovn-dbctl.c
> @@ -237,6 +237,10 @@ cleanup:
>          }
>          free(commands);
>          if (error) {
> +            for (int i = 0; i < argc; i++) {
> +                free(argv_[i]);
> +            }
> +            free(argv_);
>              ctl_fatal("%s", error);
>          }
>      }
> --
> 2.30.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

amu...@redhat.com    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to