[webkit-reviews] review denied: [Bug 63966] [EFL] Add url bar to EWebLauncher. : [Attachment 100443] Another patch using edje.

[bugzilla-daemon at webkit.org]

Lucas De Marchi <demarchi at webkit.org> has denied Ryuan Choi
<ryuan.choi at samsung.com>'s request for review:
Bug 63966: [EFL] Add url bar to EWebLauncher.
https://bugs.webkit.org/show_bug.cgi?id=63966

Attachment 100443: Another patch using edje.
https://bugs.webkit.org/attachment.cgi?id=100443&action=review

------- Additional Comments from Lucas De Marchi <demarchi at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100443&action=review


Are those "if (!app)", "if (!app || !ev | ...)" really necessary? Seems like
over protection. Just guarantee that the function cannot be called with NULL
pointers. Since most of them come from evas_object_event_*, you can be sure
they are not NULL.

> Tools/EWebLauncher/main.c:507
> +    if (!app || !ev)
> +	   return;

why?

> Tools/EWebLauncher/main.c:536
> +    if (!app || !ev)
> +	   return;

why?

> Tools/EWebLauncher/main.c:667
> +    if (!app)
> +	   return;

why?

> Tools/EWebLauncher/main.c:717
> +		   memset(full_url, 0x00, len);

You don't need this. Set only the last byte as '\0'

> Tools/EWebLauncher/main.c:747
> +	       ewk_view_uri_set(app->browser, full_url);
> +	       evas_object_focus_set(app->browser, EINA_TRUE);
> +
> +	       if (full_url)
> +		   free(full_url);

Why this is not after char *full_url?