Page MenuHomePhabricator

elm: prevent from accessing null pointer after memory allocation
ClosedPublic

Authored by woohyun on Jan 28 2019, 3:41 AM.

Details

Summary

Add null checking code just after allocating memory

Test Plan

make check

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
woohyun created this revision.Jan 28 2019, 3:41 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

woohyun requested review of this revision.Jan 28 2019, 3:41 AM
zmike requested changes to this revision.Jan 28 2019, 8:58 AM
zmike added a subscriber: zmike.

This commit log should use the elm: prefix on the first line.

This revision now requires changes to proceed.Jan 28 2019, 8:58 AM
devilhorns added inline comments.
src/lib/elementary/elm_theme.c
546

Why does this check use EINA_SAFETY and the others do not ??

woohyun added inline comments.Jan 28 2019, 6:38 PM
src/lib/elementary/elm_theme.c
546

I've just copied a code in _elm_theme_itme_finalize in the same file : )
That is, just keeping consistency in the file.

woohyun updated this revision to Diff 18979.Jan 28 2019, 6:41 PM

Change the commit message

woohyun updated this revision to Diff 18980.Jan 28 2019, 6:44 PM
woohyun retitled this revision from Prevent from accessing null pointer after memory allocation to elm: prevent from accessing null pointer after memory allocation.

update commit message

In D7801#138165, @zmike wrote:

This commit log should use the elm: prefix on the first line.

Thanks ~ I did change the commit message.

Jaehyun_Cho requested changes to this revision.Jan 28 2019, 9:23 PM

I think my second comment is required to prevent memory leak.
I am not sure about my first and my third comments.

src/lib/elementary/efl_page_transition_scroll.c
44

shouldn't the previously allocated "pi" be freed?

src/lib/elementary/elm_code_widget_text.c
70

it seems that "first" and "last" should be freed.
how about using "goto end"?

end:

free(first);
free(last);
return ret;

}

src/lib/elementary/elm_theme.c
546

shouldn't the previously allocated "cpy" be freed?

This revision now requires changes to proceed.Jan 28 2019, 9:23 PM
woohyun added inline comments.Jan 28 2019, 10:40 PM
src/lib/elementary/efl_page_transition_scroll.c
44

Allocated "pi"s will be kept in the widget, and would be freed when it is invalidated.

src/lib/elementary/elm_code_widget_text.c
70

I missed this - will update soon.

src/lib/elementary/elm_theme.c
546

Allocated "cpy" would be kept inside the destination list, and the list would be handled manually by application side.

woohyun updated this revision to Diff 18990.Jan 28 2019, 10:43 PM

Update elm_code_widget_text.c to free allocated memory

Jaehyun_Cho accepted this revision.Jan 28 2019, 10:51 PM

@zmike @devilhorns

To me, this patch is acceptable.
Can I submit this patch? :)

zmike accepted this revision.Jan 29 2019, 6:11 AM
This revision is now accepted and ready to land.Jan 29 2019, 6:11 AM
This revision was automatically updated to reflect the committed changes.