[LEDE-DEV] [PATCH] ubus: Remove unnecessary memset calls.

[Arjen de Korte]

Citeren Rosen Penev <rosenp at gmail.com>:

> I beg to differ. https://vorpus.org/blog/why-does-calloc-exist/
>
> Section 2.

I don't care about theoretical gains, benchmarks please. How much do  
you gain with these patches? I really doubt that in any of these  
patches there will be a tangible gain.

> On Tue, Nov 7, 2017 at 12:46 PM, Arjen de Korte  
> <arjen+lede at de-korte.org> wrote:
>> Citeren Rosen Penev <rosenp at gmail.com>:
>>
>>> Replace malloc+memset with calloc. Cleaner and faster in extreme
>>> situations.
>>
>>
>> Calloc is definitly *not* faster than malloc + memset. Under the hood,
>> calloc will call malloc, check if memory allocation was successful and then
>> proceed to set all allocated memory to 0. You need to check the return value
>> of malloc or calloc anyway, so this will be actually slower.
>>
>> It *may* be considered cleaner, since it will check if the memory allocation
>> succeeded before writing zeros.
>>
>>
>>> Signed-off-by: Rosen Penev <rosenp at gmail.com>
>>> ---
>>>  libubus.c  |  6 ++----
>>>  lua/ubus.c | 18 ++++++------------
>>>  2 files changed, 8 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/libubus.c b/libubus.c
>>> index 9463522..260e40f 100644
>>> --- a/libubus.c
>>> +++ b/libubus.c
>>> @@ -133,7 +133,7 @@ struct ubus_lookup_request {
>>>  static void ubus_lookup_cb(struct ubus_request *ureq, int type, struct
>>> blob_attr *msg)
>>>  {
>>>         struct ubus_lookup_request *req;
>>> -       struct ubus_object_data obj;
>>> +       struct ubus_object_data obj = {};
>>>         struct blob_attr **attr;
>>>
>>>         req = container_of(ureq, struct ubus_lookup_request, req);
>>> @@ -143,7 +143,6 @@ static void ubus_lookup_cb(struct ubus_request *ureq,
>>> int type, struct blob_attr
>>>             !attr[UBUS_ATTR_OBJTYPE])
>>>                 return;
>>>
>>> -       memset(&obj, 0, sizeof(obj));
>>>         obj.id = blob_get_u32(attr[UBUS_ATTR_OBJID]);
>>>         obj.path = blob_data(attr[UBUS_ATTR_OBJPATH]);
>>>         obj.type_id = blob_get_u32(attr[UBUS_ATTR_OBJTYPE]);
>>> @@ -220,7 +219,7 @@ int ubus_register_event_handler(struct ubus_context
>>> *ctx,
>>>                                 const char *pattern)
>>>  {
>>>         struct ubus_object *obj = &ev->obj;
>>> -       struct blob_buf b2;
>>> +       struct blob_buf b2 = {};
>>>         int ret;
>>>
>>>         if (!obj->id) {
>>> @@ -236,7 +235,6 @@ int ubus_register_event_handler(struct ubus_context
>>> *ctx,
>>>         }
>>>
>>>         /* use a second buffer, ubus_invoke() overwrites the primary one
>>> */
>>> -       memset(&b2, 0, sizeof(b2));
>>>         blob_buf_init(&b2, 0);
>>>         blobmsg_add_u32(&b2, "object", obj->id);
>>>         if (pattern)
>>> diff --git a/lua/ubus.c b/lua/ubus.c
>>> index 74a15b0..e9fd10e 100644
>>> --- a/lua/ubus.c
>>> +++ b/lua/ubus.c
>>> @@ -410,11 +410,10 @@ static int ubus_lua_load_methods(lua_State *L,
>>> struct ubus_method *m)
>>>         }
>>>
>>>         /* setup the policy pointers */
>>> -       p = malloc(sizeof(struct blobmsg_policy) * plen);
>>> +       p = calloc(plen, sizeof(struct blobmsg_policy));
>>>         if (!p)
>>>                 return 1;
>>>
>>> -       memset(p, 0, sizeof(struct blobmsg_policy) * plen);
>>>         m->policy = p;
>>>         lua_pushnil(L);
>>>         while (lua_next(L, -2) != 0) {
>>> @@ -481,26 +480,23 @@ static struct ubus_object*
>>> ubus_lua_load_object(lua_State *L)
>>>         int midx = 0;
>>>
>>>         /* setup object pointers */
>>> -       obj = malloc(sizeof(struct ubus_lua_object));
>>> +       obj = calloc(1, sizeof(struct ubus_lua_object));
>>>         if (!obj)
>>>                 return NULL;
>>>
>>> -       memset(obj, 0, sizeof(struct ubus_lua_object));
>>>         obj->o.name = lua_tostring(L, -2);
>>>
>>>         /* setup method pointers */
>>> -       m = malloc(sizeof(struct ubus_method) * mlen);
>>> -       memset(m, 0, sizeof(struct ubus_method) * mlen);
>>> +       m = calloc(1, sizeof(struct ubus_method) * mlen);
>>>         obj->o.methods = m;
>>>
>>>         /* setup type pointers */
>>> -       obj->o.type = malloc(sizeof(struct ubus_object_type));
>>> +       obj->o.type = calloc(1, sizeof(struct ubus_object_type));
>>>         if (!obj->o.type) {
>>>                 free(obj);
>>>                 return NULL;
>>>         }
>>>
>>> -       memset(obj->o.type, 0, sizeof(struct ubus_object_type));
>>>         obj->o.type->name = lua_tostring(L, -2);
>>>         obj->o.type->id = 0;
>>>         obj->o.type->methods = obj->o.methods;
>>> @@ -715,11 +711,10 @@ ubus_lua_load_event(lua_State *L)
>>>  {
>>>         struct ubus_lua_event* event = NULL;
>>>
>>> -       event = malloc(sizeof(struct ubus_lua_event));
>>> +       event = calloc(1, sizeof(struct ubus_lua_event));
>>>         if (!event)
>>>                 return NULL;
>>>
>>> -       memset(event, 0, sizeof(struct ubus_lua_event));
>>>         event->e.cb = ubus_event_handler;
>>>
>>>         /* update the he callback lookup table */
>>> @@ -818,8 +813,7 @@ ubus_lua_do_subscribe( struct ubus_context *ctx,
>>> lua_State *L, const char* targe
>>>                 lua_error( L );
>>>         }
>>>
>>> -       sub = malloc( sizeof( struct ubus_lua_subscriber ) );
>>> -       memset( sub, 0, sizeof( struct ubus_lua_subscriber ) );
>>> +       sub = calloc( 1, sizeof( struct ubus_lua_subscriber ) );
>>>         if( !sub ){
>>>                 lua_pushstring( L, "Out of memory" );
>>>                 lua_error( L );
>>
>>
>>
>>
>>
>> _______________________________________________
>> Lede-dev mailing list
>> Lede-dev at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev