[PATCH] Remove superfluous if(p!=NULL) checks around free(p); p=NULL;
Matt Turner
mattst88 at gmail.com
Wed Nov 10 07:57:28 PST 2010
On Wed, Nov 10, 2010 at 10:06 AM, Cyril Brulebois <kibi at debian.org> wrote:
> This patch has been generated by the following Coccinelle semantic patch:
>
> @@
> expression E;
> @@
> - if (E != NULL) {
> - free(E);
> (
> - E = NULL;
> |
> - E = 0;
> )
> - }
> + free(E);
> + E = NULL;
>
> Signed-off-by: Cyril Brulebois <kibi at debian.org>
> ---
> dix/ptrveloc.c | 8 +++-----
> hw/kdrive/fake/fake.c | 7 ++-----
> hw/xwin/wincmap.c | 7 ++-----
> hw/xwin/winpixmap.c | 7 ++-----
> miext/rootless/rootlessWindow.c | 6 ++----
> render/miindex.c | 14 ++++----------
> render/picture.c | 7 ++-----
> xkb/XKBAlloc.c | 6 ++----
> xkb/XKBGAlloc.c | 30 ++++++++++--------------------
> xkb/XKBMAlloc.c | 34 ++++++++++++----------------------
> 10 files changed, 41 insertions(+), 85 deletions(-)
>
> diff --git a/dix/ptrveloc.c b/dix/ptrveloc.c
> index 30e14b1..8f03321 100644
> --- a/dix/ptrveloc.c
> +++ b/dix/ptrveloc.c
> @@ -952,11 +952,9 @@ SetAccelerationProfile(
> if(profile == NULL && profile_num != PROFILE_UNINITIALIZE)
> return FALSE;
>
> - if(vel->profile_private != NULL){
> - /* Here one could free old profile-private data */
> - free(vel->profile_private);
> - vel->profile_private = NULL;
> - }
> + /* Here one could free old profile-private data */
> + free(vel->profile_private);
> + vel->profile_private = NULL;
> /* Here one could init profile-private data */
> vel->Profile = profile;
> vel->statistics.profile_number = profile_num;
> diff --git a/hw/kdrive/fake/fake.c b/hw/kdrive/fake/fake.c
> index b8306db..ba05234 100644
> --- a/hw/kdrive/fake/fake.c
> +++ b/hw/kdrive/fake/fake.c
> @@ -215,11 +215,8 @@ fakeUnmapFramebuffer (KdScreenInfo *screen)
> {
> FakePriv *priv = screen->card->driver;
> KdShadowFbFree (screen);
> - if (priv->base)
> - {
> - free (priv->base);
> - priv->base = 0;
> - }
> + free(priv->base);
> + priv->base = NULL;
> return TRUE;
> }
>
> diff --git a/hw/xwin/wincmap.c b/hw/xwin/wincmap.c
> index 9da0388..d526a92 100644
> --- a/hw/xwin/wincmap.c
> +++ b/hw/xwin/wincmap.c
> @@ -516,11 +516,8 @@ winGetPaletteDD (ScreenPtr pScreen, ColormapPtr pcmap)
> pScreen->blackPixel = 0;
>
> /* Free colormap */
> - if (ppeColors != NULL)
> - {
> - free (ppeColors);
> - ppeColors = NULL;
> - }
> + free(ppeColors);
> + ppeColors = NULL;
>
> /* Free the DC */
> if (hdc != NULL)
> diff --git a/hw/xwin/winpixmap.c b/hw/xwin/winpixmap.c
> index 050c71a..8bd8e34 100644
> --- a/hw/xwin/winpixmap.c
> +++ b/hw/xwin/winpixmap.c
> @@ -163,11 +163,8 @@ winDestroyPixmapNativeGDI (PixmapPtr pPixmap)
> if (pPixmapPriv->hBitmap) DeleteObject (pPixmapPriv->hBitmap);
>
> /* Free the bitmap info header memory */
> - if (pPixmapPriv->pbmih != NULL)
> - {
> - free (pPixmapPriv->pbmih);
> - pPixmapPriv->pbmih = NULL;
> - }
> + free(pPixmapPriv->pbmih);
> + pPixmapPriv->pbmih = NULL;
>
> /* Free the pixmap memory */
> free (pPixmap);
> diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c
> index 42ab8da..c4a32aa 100644
> --- a/miext/rootless/rootlessWindow.c
> +++ b/miext/rootless/rootlessWindow.c
> @@ -1140,10 +1140,8 @@ FinishFrameResize(WindowPtr pWin, Bool gravity, int oldX, int oldY,
> }
> }
>
> - if (gResizeDeathBits != NULL) {
> - free(gResizeDeathBits);
> - gResizeDeathBits = NULL;
> - }
> + free(gResizeDeathBits);
> + gResizeDeathBits = NULL;
>
> if (gravity) {
> pScreen->CopyWindow = gResizeOldCopyWindowProc;
> diff --git a/render/miindex.c b/render/miindex.c
> index 5e2e06c..4603136 100644
> --- a/render/miindex.c
> +++ b/render/miindex.c
> @@ -322,16 +322,10 @@ void
> miCloseIndexed (ScreenPtr pScreen,
> PictFormatPtr pFormat)
> {
> - if (pFormat->index.devPrivate)
> - {
> - free(pFormat->index.devPrivate);
> - pFormat->index.devPrivate = 0;
> - }
> - if (pFormat->index.pValues)
> - {
> - free(pFormat->index.pValues);
> - pFormat->index.pValues = 0;
> - }
> + free(pFormat->index.devPrivate);
> + pFormat->index.devPrivate = NULL;
> + free(pFormat->index.pValues);
> + pFormat->index.pValues = NULL;
> }
>
> void
> diff --git a/render/picture.c b/render/picture.c
> index 7fda6b9..896eaa7 100644
> --- a/render/picture.c
> +++ b/render/picture.c
> @@ -1391,11 +1391,8 @@ SetPictureTransform (PicturePtr pPicture,
> }
> else
> {
> - if (pPicture->transform)
> - {
> - free(pPicture->transform);
> - pPicture->transform = 0;
> - }
> + free(pPicture->transform);
> + pPicture->transform = NULL;
> }
> pPicture->serialNumber |= GC_CHANGE_SERIAL_BIT;
>
> diff --git a/xkb/XKBAlloc.c b/xkb/XKBAlloc.c
> index c52e091..bffd60f 100644
> --- a/xkb/XKBAlloc.c
> +++ b/xkb/XKBAlloc.c
> @@ -212,10 +212,8 @@ XkbNamesPtr names;
> register XkbKeyTypePtr type;
> type= map->types;
> for (i=0;i<map->num_types;i++,type++) {
> - if (type->level_names!=NULL) {
> - free(type->level_names);
> - type->level_names= NULL;
> - }
> + free(type->level_names);
> + type->level_names = NULL;
> }
> }
> }
> diff --git a/xkb/XKBGAlloc.c b/xkb/XKBGAlloc.c
> index d1adea3..3ec9eda 100644
> --- a/xkb/XKBGAlloc.c
> +++ b/xkb/XKBGAlloc.c
> @@ -50,10 +50,8 @@ _XkbFreeGeomLeafElems( Bool freeAll,
> {
> if ((freeAll)||(*elems==NULL)) {
> *num_inout= *sz_inout= 0;
> - if (*elems!=NULL) {
> - free(*elems);
> - *elems= NULL;
> - }
> + free(*elems);
> + *elems = NULL;
> return;
> }
>
> @@ -373,22 +371,16 @@ XkbDoodadPtr doodad= (XkbDoodadPtr)doodad_in;
> switch (doodad->any.type) {
> case XkbTextDoodad:
> {
> - if (doodad->text.text!=NULL) {
> - free(doodad->text.text);
> - doodad->text.text= NULL;
> - }
> - if (doodad->text.font!=NULL) {
> - free(doodad->text.font);
> - doodad->text.font= NULL;
> - }
> + free(doodad->text.text);
> + doodad->text.text = NULL;
> + free(doodad->text.font);
> + doodad->text.font = NULL;
> }
> break;
> case XkbLogoDoodad:
> {
> - if (doodad->logo.logo_name!=NULL) {
> - free(doodad->logo.logo_name);
> - doodad->logo.logo_name= NULL;
> - }
> + free(doodad->logo.logo_name);
> + doodad->logo.logo_name = NULL;
> }
> break;
> }
> @@ -434,10 +426,8 @@ XkbFreeGeometry(XkbGeometryPtr geom,unsigned which,Bool freeMap)
> if ((which&XkbGeomKeyAliasesMask)&&(geom->key_aliases!=NULL))
> XkbFreeGeomKeyAliases(geom,0,geom->num_key_aliases,TRUE);
> if (freeMap) {
> - if (geom->label_font!=NULL) {
> - free(geom->label_font);
> - geom->label_font= NULL;
> - }
> + free(geom->label_font);
> + geom->label_font = NULL;
> free(geom);
> }
> return;
> diff --git a/xkb/XKBMAlloc.c b/xkb/XKBMAlloc.c
> index e9afe31..2681ba3 100644
> --- a/xkb/XKBMAlloc.c
> +++ b/xkb/XKBMAlloc.c
> @@ -319,9 +319,9 @@ KeyCode matchingKeys[XkbMaxKeyCount],nMatchingKeys;
> return BadAlloc;
> }
> }
> - else if (type->preserve!=NULL) {
> + else {
> free(type->preserve);
> - type->preserve= NULL;
> + type->preserve = NULL;
> }
> type->map_count= map_count;
> }
> @@ -805,19 +805,13 @@ XkbClientMapPtr map;
> register int i;
> XkbKeyTypePtr type;
> for (i=0,type=map->types;i<map->num_types;i++,type++) {
> - if (type->map!=NULL) {
> - free(type->map);
> - type->map= NULL;
> - }
> - if (type->preserve!=NULL) {
> - free(type->preserve);
> - type->preserve= NULL;
> - }
> + free(type->map);
> + type->map = NULL;
> + free(type->preserve);
> + type->preserve = NULL;
> type->map_count= 0;
> - if (type->level_names!=NULL) {
> - free(type->level_names);
> - type->level_names= NULL;
> - }
> + free(type->level_names);
> + type->level_names = NULL;
> }
> }
> free(map->types);
> @@ -826,10 +820,8 @@ XkbClientMapPtr map;
> }
> }
> if (what&XkbKeySymsMask) {
> - if (map->key_sym_map!=NULL) {
> - free(map->key_sym_map);
> - map->key_sym_map= NULL;
> - }
> + free(map->key_sym_map);
> + map->key_sym_map = NULL;
> if (map->syms!=NULL) {
> free(map->syms);
> map->size_syms= map->num_syms= 0;
> @@ -862,10 +854,8 @@ XkbServerMapPtr map;
> map->explicit= NULL;
> }
> if (what&XkbKeyActionsMask) {
> - if (map->key_acts!=NULL) {
> - free(map->key_acts);
> - map->key_acts= NULL;
> - }
> + free(map->key_acts);
> + map->key_acts = NULL;
> if (map->acts!=NULL) {
> free(map->acts);
> map->num_acts= map->size_acts= 0;
> --
> 1.7.2.3
I'm not so sure about this. Do you know for sure that nothing depends
on setting the value of the freed pointer to NULL or 0 afterwards?
I think this information is used in some places, ugh.
And good grief. Has anyone kept count of how many various incarnations
of this patch there have been?
if (p) free(p);
if (p != NULL) free(p);
then with { }
then add p = NULL or p = 0.
Is it possible to write a coccinelle patch to catch all of these cases?
Thanks,
Matt
More information about the xorg-devel
mailing list