[PATCH libICE 10/13] add check for malloc

[Emil Velikov]

On 18 October 2017 at 17:04, walter harms <wharms at bfs.de> wrote:
> From ea066aa04dd118187ca0289053bc4ca5caa0a4a8 Mon Sep 17 00:00:00 2001
>
>
> fix a potential null pointer deference error
> convert malloc() to calloc() to have valid
> null pointers on error. so we can release
> already allocated memory
>
I'd reword this to something like:
Handle memory allocation failure, cleaning up after ourselves. Use
calloc the base struct since we can safely pass NULL pointers to
free() in the teardown path..

> Signed-off-by: Walter Harms <wharms at bfs.de>
> ---
>  src/register.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/src/register.c b/src/register.c
> index 833714b..2417dd7 100644
> --- a/src/register.c
> +++ b/src/register.c
> @@ -67,7 +67,9 @@ IceRegisterForProtocolSetup (
>
>      if (i <= _IceLastMajorOpcode)
>      {
> -       p = _IceProtocols[i - 1].orig_client = malloc (sizeof(_IcePoProtocol));
> +        p = _IceProtocols[i - 1].orig_client = calloc (1,sizeof(_IcePoProtocol));
Please add the infamous space after , through the series.

> +       if (!p)
> +         return (-1);
Move the if !p check, just before dereferencing it - this way we need
only one ;-)

>         opcodeRet = i;
>      }
>      else if (_IceLastMajorOpcode == 255 ||
> @@ -82,7 +84,9 @@ IceRegisterForProtocolSetup (
>             strdup(protocolName);
>
strdup can fail -

if (!p->vendor || !p->vendor)
     goto out_of_memory;


>         p = _IceProtocols[_IceLastMajorOpcode].orig_client =
> -           malloc (sizeof (_IcePoProtocol));
> +           calloc (1,sizeof (_IcePoProtocol));
> +       if (!p)
> +         return (-1);
>
>         _IceProtocols[_IceLastMajorOpcode].accept_client = NULL;
>
> @@ -95,15 +99,20 @@ IceRegisterForProtocolSetup (
>      p->version_count = versionCount;
>
>      p->version_recs = malloc (versionCount * sizeof (IcePoVersionRec));
> +    if (!p->version_recs)
> +        goto out_of_memory;
> +
>      memcpy (p->version_recs, versionRecs,
>         versionCount * sizeof (IcePoVersionRec));
>
>      if ((p->auth_count = authCount) > 0)
>      {
>         p->auth_names = malloc (authCount * sizeof (char *));
> -
> +       if (!p->auth_names);
> +            goto out_of_memory;
>         p->auth_procs = malloc (authCount * sizeof (IcePoAuthProc));
> -
> +       if (!p->auth_names);
> +            goto out_of_memory;
I'd just use

if (!p->auth_names || !p->auth_procs);
     goto out_of_memory;

Exact same suggestions apply for IceRegisterForProtocolReply()

-Emil