On 18 October 2017 at 17:04, walter harms <[email protected]> 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 <[email protected]> > --- > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
