Page MenuHomePhabricator

edje: Add null check
Needs RevisionPublic

Authored by subodh6129 on Jan 24 2020, 6:44 AM.

Details

Reviewers
cedric
bu5hm4n
Summary

If style is null, textblock style
is being created unneccessary. Patch fixes it.

@fix

Test Plan

NA

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15666
Build 10694: arc lint + arc unit
subodh6129 created this revision.Jan 24 2020, 6:44 AM
subodh6129 requested review of this revision.Jan 24 2020, 6:44 AM

I think you should better use the macro EINA_SAFETY_ON_NULL_RETURN here, and probably before GET_REAL_PART_ON_FAIL_RETURN().

cedric requested changes to this revision.Jan 24 2020, 9:25 AM

I agree with @ProhtMeyhet .

This revision now requires changes to proceed.Jan 24 2020, 9:25 AM

@cedric

Isn't the EINA_UNUSED wrong on the function argument Eo *obj ? Because the macro GET_REAL_PART_ON_FAIL_RETURN looks like this and it includes obj

#define GET_REAL_PART_ON_FAIL_RETURN(x) Edje_Real_Part *rp;\
                                        Edje *ed;\
                                        ed = _edje_fetch(obj);\
                                        if ((!ed) || (!part)) return x;\
                                        rp = _edje_real_part_recursive_get(&ed, part);\
                                        if (!rp) return x;\
subodh6129 updated this revision to Diff 28510.Jan 26 2020, 9:58 PM

Fix review comments.

bu5hm4n requested changes to this revision.Jan 29 2020, 12:34 AM
bu5hm4n added a subscriber: bu5hm4n.

Sounds like a good idea, however, now the tests dont pass anymore :/

This revision now requires changes to proceed.Jan 29 2020, 12:34 AM

Sounds like a good idea, however, now the tests dont pass anymore :/

which one? I just completely rebuild and efl_suite and edje_suite run fine with this patch.

elementary_test fails because errors are now printed.

elementary_test fails because errors are now printed.

elementary_test fails for me without this patch with SEGFAULTS with the state of efl commit 509ad380832736361e01510991d27c67f8d10664:

 ./elementary_suite
Running suite(s): Elementary_Init
100%: Checks: 1, Failures: 0, Errors: 0
Running suite(s): Elementary
Running suite(s): Elementary
100%: Checks: 1, Failures: 0, Errors: 0
Running suite(s): Elementary
0%: Checks: 3, Failures: 0, Errors: 3
../src/tests/elementary/elm_test_colorselector.c:9:E:elm_colorselector:elm_colorselector_legacy_type_check:0: (after this point) Received signal 11 (Segmentation fault)
../src/tests/elementary/elm_test_colorselector.c:29:E:elm_colorselector:elm_colorselector_palette:0: (after this point) Received signal 11 (Segmentation fault)
../src/tests/elementary/elm_test_colorselector.c:54:E:elm_colorselector:elm_atspi_role_get:0: (after this point) Received signal 11 (Segmentation fault)

Oh, now that is interesting - can you create a ticket, and share the backtrace there ?

I compiled with ggc-9.2

gcc --version
gcc (SUSE Linux) 9.2.1 20200109 [gcc-9-branch revision 280039]

so c245b576aad09ac5faeb800de7f7c4fef87c6363 shouldn't apply.

Oh, now that is interesting - can you create a ticket, and share the backtrace there ?

how do I get a backtrace of one of these tests from elementary_test? Just run in gdb, or is there a special option, or can I build these tests into separate binarys?

ahhhrg, just running it in gdb gives me a bus error:

gdb ./elementary_suite
GNU gdb (GDB; openSUSE Tumbleweed) 8.3.1
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-suse-linux".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://bugs.opensuse.org/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./elementary_suite...
(gdb) run
Starting program: /media/media4/compile/e/efl/build/src/tests/elementary/elementary_suite
Bus error (core dumped)

Try under valgrind with --trace-children=yes and EFL_RUN_IN_TREE=1.