[PATCH weston v2 3/3] ivi-shell: use zalloc instead of calloc
Yong Bakos
junk at humanoriented.com
Thu Jul 28 17:23:00 UTC 2016
Hi Emre,
> On Jul 28, 2016, at 7:14 AM, Ucan, Emre (ADITG/SW1) <eucan at de.adit-jv.com> wrote:
>
> v2 changes:
> - use xzalloc
> - add an explicit include of xalloc.h in any .c file
> that uses xzalloc.
>
> Signed-off-by: Emre Ucan <eucan at de.adit-jv.com>
Is the intent of this patch to cause an exit() when memory allocation fails?
See my comments inline below.
Respectfully,
yong
> ---
> ivi-shell/hmi-controller.c | 35 ++++++++------------
> ivi-shell/input-panel-ivi.c | 6 ++--
> ivi-shell/ivi-layout-transition.c | 62 +++++++----------------------------
> ivi-shell/ivi-layout.c | 65 +++++++------------------------------
> 4 files changed, 38 insertions(+), 130 deletions(-)
>
> diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
> index 6ef2cd6..bc2f2cf 100644
> --- a/ivi-shell/hmi-controller.c
> +++ b/ivi-shell/hmi-controller.c
> @@ -155,13 +155,6 @@ controller_module_init(struct weston_compositor *ec,
> /*****************************************************************************
> * local functions
> ****************************************************************************/
> -static void *
> -mem_alloc(size_t size, char *file, int32_t line)
> -{
> - return fail_on_null(calloc(1, size), size, file, line);
> -}
> -
> -#define MEM_ALLOC(s) mem_alloc((s),__FILE__,__LINE__)
>
> static int32_t
> is_surf_in_ui_widget(struct hmi_controller *hmi_ctrl,
> @@ -224,8 +217,8 @@ mode_divided_into_tiling(struct hmi_controller *hmi_ctrl,
> int32_t surf_num = 0;
> int32_t idx = 0;
>
> - surfaces = MEM_ALLOC(sizeof(*surfaces) * surface_length);
> - new_order = MEM_ALLOC(sizeof(*surfaces) * surface_length);
> + surfaces = xzalloc(sizeof(*surfaces) * surface_length);
> + new_order = xzalloc(sizeof(*surfaces) * surface_length);
>
> for (i = 0; i < surface_length; i++) {
> ivisurf = pp_surface[i];
> @@ -299,8 +292,8 @@ mode_divided_into_sidebyside(struct hmi_controller *hmi_ctrl,
> int32_t surf_num = 0;
> int32_t idx = 0;
>
> - surfaces = MEM_ALLOC(sizeof(*surfaces) * surface_length);
> - new_order = MEM_ALLOC(sizeof(*surfaces) * surface_length);
> + surfaces = xzalloc(sizeof(*surfaces) * surface_length);
> + new_order = xzalloc(sizeof(*surfaces) * surface_length);
>
> for (i = 0; i < surface_length; i++) {
> ivisurf = pp_surface[i];
> @@ -364,7 +357,7 @@ mode_fullscreen_someone(struct hmi_controller *hmi_ctrl,
> int32_t surf_num = 0;
> struct ivi_layout_surface **surfaces;
>
> - surfaces = MEM_ALLOC(sizeof(*surfaces) * surface_length);
> + surfaces = xzalloc(sizeof(*surfaces) * surface_length);
>
> for (i = 0; i < surface_length; i++) {
> ivisurf = pp_surface[i];
> @@ -414,7 +407,7 @@ mode_random_replace(struct hmi_controller *hmi_ctrl,
> int32_t i = 0;
> int32_t layer_idx = 0;
>
> - layers = MEM_ALLOC(sizeof(*layers) * hmi_ctrl->screen_num);
> + layers = xzalloc(sizeof(*layers) * hmi_ctrl->screen_num);
>
> wl_list_for_each(application_layer, layer_list, link) {
> layers[layer_idx] = application_layer;
> @@ -676,7 +669,7 @@ set_notification_configure_surface(struct wl_listener *listener, void *data)
> static struct hmi_server_setting *
> hmi_server_setting_create(struct weston_compositor *ec)
> {
> - struct hmi_server_setting *setting = MEM_ALLOC(sizeof(*setting));
> + struct hmi_server_setting *setting = xzalloc(sizeof *setting);
> struct weston_config *config = ec->config;
> struct weston_config_section *shell_section = NULL;
>
> @@ -767,7 +760,7 @@ hmi_controller_create(struct weston_compositor *ec)
> {
> struct link_layer *tmp_link_layer = NULL;
> int32_t panel_height = 0;
> - struct hmi_controller *hmi_ctrl = MEM_ALLOC(sizeof(*hmi_ctrl));
> + struct hmi_controller *hmi_ctrl = xzalloc(sizeof *hmi_ctrl);
> struct hmi_controller_layer *base_layer = NULL;
> struct hmi_controller_layer *application_layer = NULL;
> struct weston_output *output;
> @@ -783,7 +776,7 @@ hmi_controller_create(struct weston_compositor *ec)
> /* init base ivi_layer*/
> wl_list_init(&hmi_ctrl->base_layer_list);
> wl_list_for_each(output, &ec->output_list, link) {
> - base_layer = MEM_ALLOC(1 * sizeof(struct hmi_controller_layer));
> + base_layer = xzalloc(sizeof *base_layer);
> base_layer->x = 0;
> base_layer->y = 0;
> base_layer->width = output->current_mode->width;
> @@ -803,7 +796,7 @@ hmi_controller_create(struct weston_compositor *ec)
> /* init application ivi_layer */
> wl_list_init(&hmi_ctrl->application_layer_list);
> wl_list_for_each(output, &ec->output_list, link) {
> - application_layer = MEM_ALLOC(1 * sizeof(struct hmi_controller_layer));
> + application_layer = xzalloc(sizeof *application_layer);
> application_layer->x = 0;
> application_layer->y = 0;
> application_layer->width = output->current_mode->width;
> @@ -838,7 +831,7 @@ hmi_controller_create(struct weston_compositor *ec)
>
>
> wl_list_init(&hmi_ctrl->workspace_fade.layer_list);
> - tmp_link_layer = MEM_ALLOC(sizeof(*tmp_link_layer));
> + tmp_link_layer = xzalloc(sizeof(*tmp_link_layer));
> tmp_link_layer->layout_layer =
> hmi_ctrl->workspace_background_layer.ivilayer;
> wl_list_insert(&hmi_ctrl->workspace_fade.layer_list,
> @@ -1233,7 +1226,7 @@ ivi_hmi_controller_add_launchers(struct hmi_controller *hmi_ctrl,
> ivi_layout_interface->layer_set_visibility(hmi_ctrl->workspace_layer.ivilayer,
> false);
>
> - tmp_link_layer = MEM_ALLOC(sizeof(*tmp_link_layer));
> + tmp_link_layer = xzalloc(sizeof *tmp_link_layer);
> tmp_link_layer->layout_layer = hmi_ctrl->workspace_layer.ivilayer;
> wl_list_insert(&hmi_ctrl->workspace_fade.layer_list,
> &tmp_link_layer->link);
> @@ -1721,7 +1714,7 @@ create_workspace_pointer_move(struct weston_pointer *pointer,
> struct wl_resource* resource)
> {
> struct pointer_move_grab *pnt_move_grab =
> - MEM_ALLOC(sizeof(*pnt_move_grab));
> + xzalloc(sizeof *pnt_move_grab);
>
> pnt_move_grab->base.resource = resource;
> move_grab_init_workspace(&pnt_move_grab->move, pointer->grab_x,
> @@ -1735,7 +1728,7 @@ create_workspace_touch_move(struct weston_touch *touch,
> struct wl_resource* resource)
> {
> struct touch_move_grab *tch_move_grab =
> - MEM_ALLOC(sizeof(*tch_move_grab));
> + xzalloc(sizeof *tch_move_grab);
>
> tch_move_grab->base.resource = resource;
> tch_move_grab->is_active = 1;
> diff --git a/ivi-shell/input-panel-ivi.c b/ivi-shell/input-panel-ivi.c
> index 581b56b..a563e31 100644
> --- a/ivi-shell/input-panel-ivi.c
> +++ b/ivi-shell/input-panel-ivi.c
> @@ -35,6 +35,7 @@
> #include "input-method-unstable-v1-server-protocol.h"
> #include "ivi-layout-private.h"
> #include "shared/helpers.h"
> +#include "shared/xalloc.h"
>
> struct input_panel_surface {
> struct wl_resource *resource;
> @@ -236,10 +237,7 @@ create_input_panel_surface(struct ivi_shell *shell,
> {
> struct input_panel_surface *input_panel_surface;
>
> - input_panel_surface = calloc(1, sizeof *input_panel_surface);
> - if (!input_panel_surface)
> - return NULL;
> -
> + input_panel_surface = xzalloc(sizeof *input_panel_surface);
This seems like a change in behavior. When calloc fails, this function
will return NULL, triggering wl_resource_post_error at all call sites.
After this change, when xzalloc fails, fail_on_null will call exit().
Just pointing this out in case it's not what you intended. (And if it is
intentional, it should be explained in the commit message.)
> surface->configure = input_panel_configure;
> surface->configure_private = input_panel_surface;
> weston_surface_set_label_func(surface, input_panel_get_label);
> diff --git a/ivi-shell/ivi-layout-transition.c b/ivi-shell/ivi-layout-transition.c
> index 989ba71..1175743 100644
> --- a/ivi-shell/ivi-layout-transition.c
> +++ b/ivi-shell/ivi-layout-transition.c
> @@ -35,6 +35,8 @@
> #include "ivi-layout-export.h"
> #include "ivi-layout-private.h"
>
> +#include "shared/xalloc.h"
> +
> struct ivi_layout_transition;
>
> typedef void (*ivi_layout_transition_frame_func)(
> @@ -185,12 +187,7 @@ ivi_layout_transition_set_create(struct weston_compositor *ec)
> struct ivi_layout_transition_set *transitions;
> struct wl_event_loop *loop;
>
> - transitions = malloc(sizeof(*transitions));
> - if (transitions == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - return NULL;
> - }
> -
> + transitions = xzalloc(sizeof *transitions);
Same per my comment above. Although, in this case, the
ivi_layout_transition_set_create call in ivi_layout_init_with_compositor
doesn't check for a NULL return anyway (!).
And, same for the remaining occurrences in this patch.
> wl_list_init(&transitions->transition_list);
>
> loop = wl_display_get_event_loop(ec->wl_display);
> @@ -207,12 +204,7 @@ layout_transition_register(struct ivi_layout_transition *trans)
> struct ivi_layout *layout = get_instance();
> struct transition_node *node;
>
> - node = malloc(sizeof(*node));
> - if (node == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - return false;
> - }
> -
> + node = xzalloc(sizeof *node);
> node->transition = trans;
> wl_list_insert(&layout->pending_transition_list, &node->link);
> return true;
> @@ -258,13 +250,9 @@ layout_transition_destroy(struct ivi_layout_transition *transition)
> static struct ivi_layout_transition *
> create_layout_transition(void)
> {
> - struct ivi_layout_transition *transition = malloc(sizeof(*transition));
> -
> - if (transition == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - return NULL;
> - }
> + struct ivi_layout_transition *transition;
>
> + transition = xzalloc(sizeof *transition);
> transition->type = IVI_LAYOUT_TRANSITION_MAX;
> transition->time_start = 0;
> transition->time_duration = 300; /* 300ms */
> @@ -492,13 +480,7 @@ create_fade_view_transition(
> if (transition == NULL)
> return NULL;
>
> - data = malloc(sizeof(*data));
> - if (data == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - free(transition);
> - return NULL;
> - }
> -
> + data = xzalloc(sizeof *data);
> transition->type = IVI_LAYOUT_TRANSITION_VIEW_FADE;
> transition->is_transition_func = (ivi_layout_is_transition_func)is_transition_fade_view_func;
>
> @@ -586,12 +568,7 @@ ivi_layout_transition_visibility_on(struct ivi_layout_surface *surface,
> if (is_visible)
> return;
>
> - user_data = malloc(sizeof(*user_data));
> - if (user_data == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - return;
> - }
> -
> + user_data = xzalloc(sizeof *user_data);
> user_data->alpha = wl_fixed_to_double(dest_alpha);
>
> create_visibility_transition(surface,
> @@ -644,12 +621,7 @@ ivi_layout_transition_visibility_off(struct ivi_layout_surface *surface,
> return;
> }
>
> - user_data = malloc(sizeof(*user_data));
> - if (user_data == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - return;
> - }
> -
> + user_data = xzalloc(sizeof *user_data);
> user_data->alpha = wl_fixed_to_double(start_alpha);
>
> create_visibility_transition(surface,
> @@ -725,13 +697,7 @@ create_move_layer_transition(
> if (transition == NULL)
> return NULL;
>
> - data = malloc(sizeof(*data));
> - if (data == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - free(transition);
> - return NULL;
> - }
> -
> + data = xzalloc(sizeof *data);
> transition->type = IVI_LAYOUT_TRANSITION_LAYER_MOVE;
> transition->is_transition_func = (ivi_layout_is_transition_func)is_transition_move_layer_func;
>
> @@ -871,13 +837,7 @@ ivi_layout_transition_fade_layer(
> if (transition == NULL)
> return;
>
> - data = malloc(sizeof(*data));
> - if (data == NULL) {
> - weston_log("%s: memory allocation fails\n", __func__);
> - free(transition);
> - return;
> - }
> -
> + data = xzalloc(sizeof *data);
> transition->type = IVI_LAYOUT_TRANSITION_LAYER_FADE;
> transition->is_transition_func = (ivi_layout_is_transition_func)is_transition_fade_layer_func;
>
> diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
> index 646eb05..48bec9d 100644
> --- a/ivi-shell/ivi-layout.c
> +++ b/ivi-shell/ivi-layout.c
> @@ -68,6 +68,7 @@
>
> #include "shared/helpers.h"
> #include "shared/os-compatibility.h"
> +#include "shared/xalloc.h"
>
> #define max(a, b) ((a) > (b) ? (a) : (b))
>
> @@ -162,12 +163,7 @@ ivi_view_create(struct ivi_layout_layer *ivilayer,
> {
> struct ivi_layout_view *ivi_view;
>
> - ivi_view = calloc(1, sizeof *ivi_view);
> - if (ivi_view == NULL) {
> - weston_log("fails to allocate memory\n");
> - return NULL;
> - }
> -
> + ivi_view = xzalloc(sizeof *ivi_view);
> ivi_view->view = weston_view_create(ivisurf->surface);
> if (ivi_view->view == NULL) {
> weston_log("fails to allocate memory\n");
> @@ -259,11 +255,7 @@ create_screen(struct weston_compositor *ec)
> struct weston_output *output = NULL;
>
> wl_list_for_each(output, &ec->output_list, link) {
> - iviscrn = calloc(1, sizeof *iviscrn);
> - if (iviscrn == NULL) {
> - weston_log("fails to allocate memory\n");
> - continue;
> - }
> + iviscrn = xzalloc(sizeof *iviscrn);
>
> iviscrn->layout = layout;
>
> @@ -1112,12 +1104,7 @@ ivi_layout_get_screens_under_layer(struct ivi_layout_layer *ivilayer,
>
> if (length != 0) {
> /* the Array must be free by module which called this function */
> - *ppArray = calloc(length, sizeof(struct weston_output *));
> - if (*ppArray == NULL) {
> - weston_log("fails to allocate memory\n");
> - return IVI_FAILED;
> - }
> -
> + *ppArray = xzalloc(sizeof(struct weston_output *) * length);
> (*ppArray)[n++] = ivilayer->on_screen->output;
> }
>
> @@ -1143,11 +1130,7 @@ ivi_layout_get_layers(int32_t *pLength, struct ivi_layout_layer ***ppArray)
>
> if (length != 0) {
> /* the Array must be freed by module which called this function */
> - *ppArray = calloc(length, sizeof(struct ivi_layout_layer *));
> - if (*ppArray == NULL) {
> - weston_log("fails to allocate memory\n");
> - return IVI_FAILED;
> - }
> + *ppArray = xzalloc(sizeof(struct ivi_layout_layer *) * length);
>
> wl_list_for_each(ivilayer, &layout->layer_list, link) {
> (*ppArray)[n++] = ivilayer;
> @@ -1179,11 +1162,7 @@ ivi_layout_get_layers_on_screen(struct weston_output *output,
>
> if (length != 0) {
> /* the Array must be freed by module which called this function */
> - *ppArray = calloc(length, sizeof(struct ivi_layout_layer *));
> - if (*ppArray == NULL) {
> - weston_log("fails to allocate memory\n");
> - return IVI_FAILED;
> - }
> + *ppArray = xzalloc(sizeof(struct ivi_layout_layer *) * length);
>
> wl_list_for_each(ivilayer, &iviscrn->order.layer_list, order.link) {
> (*ppArray)[n++] = ivilayer;
> @@ -1212,11 +1191,7 @@ ivi_layout_get_layers_under_surface(struct ivi_layout_surface *ivisurf,
> if (!wl_list_empty(&ivisurf->view_list)) {
> /* the Array must be free by module which called this function */
> length = wl_list_length(&ivisurf->view_list);
> - *ppArray = calloc(length, sizeof(struct ivi_layout_layer *));
> - if (*ppArray == NULL) {
> - weston_log("fails to allocate memory\n");
> - return IVI_FAILED;
> - }
> + *ppArray = xzalloc(sizeof(struct ivi_layout_layer *) * length);
>
> wl_list_for_each_reverse(ivi_view, &ivisurf->view_list, surf_link) {
> if (ivi_view_is_rendered(ivi_view))
> @@ -1254,11 +1229,7 @@ ivi_layout_get_surfaces(int32_t *pLength, struct ivi_layout_surface ***ppArray)
>
> if (length != 0) {
> /* the Array must be freed by module which called this function */
> - *ppArray = calloc(length, sizeof(struct ivi_layout_surface *));
> - if (*ppArray == NULL) {
> - weston_log("fails to allocate memory\n");
> - return IVI_FAILED;
> - }
> + *ppArray = xzalloc(sizeof(struct ivi_layout_surface *) * length);
>
> wl_list_for_each(ivisurf, &layout->surface_list, link) {
> (*ppArray)[n++] = ivisurf;
> @@ -1288,11 +1259,7 @@ ivi_layout_get_surfaces_on_layer(struct ivi_layout_layer *ivilayer,
>
> if (length != 0) {
> /* the Array must be freed by module which called this function */
> - *ppArray = calloc(length, sizeof(struct ivi_layout_surface *));
> - if (*ppArray == NULL) {
> - weston_log("fails to allocate memory\n");
> - return IVI_FAILED;
> - }
> + *ppArray = xzalloc(sizeof(struct ivi_layout_surface *) * length);
>
> wl_list_for_each(ivi_view, &ivilayer->order.view_list, order_link) {
> (*ppArray)[n++] = ivi_view->ivisurf;
> @@ -1318,12 +1285,7 @@ ivi_layout_layer_create_with_dimension(uint32_t id_layer,
> return ivilayer;
> }
>
> - ivilayer = calloc(1, sizeof *ivilayer);
> - if (ivilayer == NULL) {
> - weston_log("fails to allocate memory\n");
> - return NULL;
> - }
> -
> + ivilayer = xzalloc(sizeof *ivilayer);
> ivilayer->ref_count = 1;
> wl_signal_init(&ivilayer->property_changed);
> ivilayer->layout = layout;
> @@ -1967,12 +1929,7 @@ ivi_layout_surface_create(struct weston_surface *wl_surface,
> }
> }
>
> - ivisurf = calloc(1, sizeof *ivisurf);
> - if (ivisurf == NULL) {
> - weston_log("fails to allocate memory\n");
> - return NULL;
> - }
> -
> + ivisurf = xzalloc(sizeof *ivisurf);
> wl_signal_init(&ivisurf->property_changed);
> ivisurf->id_surface = id_surface;
> ivisurf->layout = layout;
> --
> 1.7.9.5
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list