On Mon, Jul 11, 2022 at 6:16 AM Ales Musil <[email protected]> wrote: > > Hi Igor, > > thank you for the patch. Yeah I was not quick enough to reply on the PR :) > > Acked-by: Ales Musil <[email protected]>
Thanks for the patch. Can you please split this patch into 2 patches. Patch 1 to fix the memory leak in utilities/ovn-dbctl.c and the patch 2 with the other changes. This way we can backport the memory fix patch to the other branches. Numan > > > On Mon, Jul 11, 2022 at 1:10 PM Igor Zhukov <[email protected]> wrote: > > > From: Igor Zhukov <[email protected]> > > > > 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 <[email protected]> > > Acked-by: Dumitru Ceara <[email protected]> > > 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 > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] IM: amusil > <https://red.ht/sig> > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
