On Tue, Sep 18, 2018 at 11:58 AM, Ou Changkun <[email protected]> wrote:
> Hi Guacamole developers, > > I just noticed a unclear code behavior in > guacamole-server/src/libguac/user.c: > > ===== > (….) > guac_user* user = calloc(1, sizeof(guac_user)); > int i; > > /* Generate ID */ > user->user_id = guac_generate_id(GUAC_USER_ID_PREFIX); > (….) > ===== > > Still in master branch: https://github.com/apache/guac > amole-server/blob/332e187813595fc2e769f3e29c0582b7ec726ea1/s > rc/libguac/user.c#L41 > > Further, its caller also not verify if guac_user_alloc() returns NULL user: > > ===== > /* Create skeleton user */ > guac_user* user = guac_user_alloc(); > user->socket = socket; > user->client = client; > user->owner = params->owner; > ===== > > Location: https://github.com/apache/guacamole-server/blob/67680bd2d51e > 7949453f0f7ffc7f4234a1136715/src/guacd/proc.c#L92 > > I am wondering weather this is intentional or not? > Should the `calloc` call be verified if returns NULL pointer? > It would be good practice to always check calls to malloc() and calloc(), yes. This isn't done 100% across the board mainly because OS kernels commonly use an opportunistic allocation strategy. In those cases, malloc() and friends will always return non-NULL, and thus software which religiously checks for NULL will still crash / be killed if such a system is under sufficient memory pressure. It seems accessing NULL struct pointer members is an > undefined behavior? Am I missing something here? > It would be better to check for NULL. Lacking that check, the result isn't undefined - a segfault will occur upon the attempt to dereference NULL. - Mike
