[Nouveau] [PATCH] Reflow logic to make it easier to follow

[Francisco Jerez]

Tomas Carnecky <tom at dbservice.com> writes:

> The control flow was:
>
> if (!y) {
>   ppix = ...
> }
>
> if (y) {
>   ...
> } else if (x) {
>   use ppix for something
> } else {
>   use ppix for something
> }
>
> Merge the if(!y) block with the two else branches. This avoids
> a false-positive in the clang static analyzer, it can't know that
> !y and x are mutually exclusive.
>
> The result looks something like this:
>
> if (y) {
>   ...
> } else {
>   ppix = ...
>   if (x) {
>     use ppix for something
>   } else {
>     use ppix for something
>   }
> }

This patch makes the code go above the 80-column limit, that's probably
the reason it was structured that way.

>
> Signed-off-by: Tomas Carnecky <tom at dbservice.com>
> ---
>
> This patch actually compiles. There are still a couple clang warnings, but
> those are dead assignments and one dead initialization. If there is interest
> I can send patches for those as well. The clang results can be viewed at
> http://stuff.caurea.org/clang-static-analyzer/nouveau
>
Sure, patches are welcome. I wouldn't bother with the warnings found in
the overlay code though, those functions are currently unused and
they'll eventually be moved to the kernel.

>  src/nouveau_xv.c |  114 ++++++++++++++++++++++++++----------------------------
>  1 files changed, 55 insertions(+), 59 deletions(-)
>
> diff --git a/src/nouveau_xv.c b/src/nouveau_xv.c
> index 4437aa6..eb43207 100644
> --- a/src/nouveau_xv.c
> +++ b/src/nouveau_xv.c
> @@ -920,7 +920,6 @@ NVPutImage(ScrnInfoPtr pScrn, short src_x, short src_y, short drw_x,
>  {
>  	NVPortPrivPtr pPriv = (NVPortPrivPtr)data;
>  	NVPtr pNv = NVPTR(pScrn);
> -	PixmapPtr ppix;
>  	/* source box */
>  	INT32 xa = 0, xb = 0, ya = 0, yb = 0;
>  	/* size to allocate in VRAM and in GART respectively */
> @@ -1249,11 +1248,29 @@ CPU_copy:
>  	if (pPriv->currentHostBuffer != NO_PRIV_HOST_BUFFER_AVAILABLE)
>  		pPriv->currentHostBuffer ^= 1;
>  
> -	/* If we're not using the hw overlay, we're rendering into a pixmap
> -	 * and need to take a couple of additional steps...
> -	 */
> -	if (!(action_flags & USE_OVERLAY)) {
> -		ppix = NVGetDrawablePixmap(pDraw);
> +	if (action_flags & USE_OVERLAY) {
> +		if (pNv->Architecture == NV_ARCH_04) {
> +			NV04PutOverlayImage(pScrn, pPriv->video_mem, offset,
> +					    id, dstPitch, &dstBox, 0, 0,
> +					    xb, yb, npixels, nlines,
> +					    src_w, src_h, drw_w, drw_h,
> +					    clipBoxes);
> +		} else {
> +			NV10PutOverlayImage(pScrn, pPriv->video_mem, offset,
> +					    uv_offset, id, dstPitch, &dstBox,
> +					    0, 0, xb, yb,
> +					    npixels, nlines, src_w, src_h,
> +					    drw_w, drw_h, clipBoxes);
> +		}
> +
> +		pPriv->currentBuffer ^= 1;
> +	} else {
> +		int ret = BadImplementation;
> +
> +		/* If we're not using the hw overlay, we're rendering into a pixmap
> +		 * and need to take a couple of additional steps...
> +		 */
> +		PixmapPtr ppix = NVGetDrawablePixmap(pDraw);
>  
>  		/* Ensure pixmap is in offscreen memory */
>  		pNv->exa_force_cp = TRUE;
> @@ -1274,69 +1291,48 @@ CPU_copy:
>  			dstBox.y2 -= ppix->screen_y;
>  		}
>  #endif
> -	}
> -
> -	if (action_flags & USE_OVERLAY) {
> -		if (pNv->Architecture == NV_ARCH_04) {
> -			NV04PutOverlayImage(pScrn, pPriv->video_mem, offset,
> -					    id, dstPitch, &dstBox, 0, 0,
> -					    xb, yb, npixels, nlines,
> -					    src_w, src_h, drw_w, drw_h,
> -					    clipBoxes);
> -		} else {
> -			NV10PutOverlayImage(pScrn, pPriv->video_mem, offset,
> -					    uv_offset, id, dstPitch, &dstBox,
> -					    0, 0, xb, yb,
> -					    npixels, nlines, src_w, src_h,
> -					    drw_w, drw_h, clipBoxes);
> -		}
>  
> -		pPriv->currentBuffer ^= 1;
> -	} else 
> -	if (action_flags & USE_TEXTURE) {
> -		int ret = BadImplementation;
>  
> -		if (pNv->Architecture == NV_ARCH_30) {
> -			ret = NV30PutTextureImage(pScrn, pPriv->video_mem,
> -						  offset, uv_offset,
> -						  id, dstPitch, &dstBox, 0, 0,
> -						  xb, yb, npixels, nlines,
> -						  src_w, src_h, drw_w, drw_h,
> -						  clipBoxes, ppix, pPriv);
> -		} else
> -		if (pNv->Architecture == NV_ARCH_40) {
> -			ret = NV40PutTextureImage(pScrn, pPriv->video_mem, 
> -						  offset, uv_offset,
> -						  id, dstPitch, &dstBox, 0, 0,
> -						  xb, yb, npixels, nlines,
> -						  src_w, src_h, drw_w, drw_h,
> -						  clipBoxes, ppix, pPriv);
> -		} else
> -		if (pNv->Architecture == NV_ARCH_50) {
> -			ret = nv50_xv_image_put(pScrn, pPriv->video_mem,
> -						offset, uv_offset,
> -						id, dstPitch, &dstBox, 0, 0,
> -						xb, yb, npixels, nlines,
> -						src_w, src_h, drw_w, drw_h,
> -						clipBoxes, ppix, pPriv);
> +		if (action_flags & USE_TEXTURE) {
> +			if (pNv->Architecture == NV_ARCH_30) {
> +				ret = NV30PutTextureImage(pScrn, pPriv->video_mem,
> +							  offset, uv_offset,
> +							  id, dstPitch, &dstBox, 0, 0,
> +							  xb, yb, npixels, nlines,
> +							  src_w, src_h, drw_w, drw_h,
> +							  clipBoxes, ppix, pPriv);
> +			} else
> +			if (pNv->Architecture == NV_ARCH_40) {
> +				ret = NV40PutTextureImage(pScrn, pPriv->video_mem, 
> +							  offset, uv_offset,
> +							  id, dstPitch, &dstBox, 0, 0,
> +							  xb, yb, npixels, nlines,
> +							  src_w, src_h, drw_w, drw_h,
> +							  clipBoxes, ppix, pPriv);
> +			} else
> +			if (pNv->Architecture == NV_ARCH_50) {
> +				ret = nv50_xv_image_put(pScrn, pPriv->video_mem,
> +							offset, uv_offset,
> +							id, dstPitch, &dstBox, 0, 0,
> +							xb, yb, npixels, nlines,
> +							src_w, src_h, drw_w, drw_h,
> +							clipBoxes, ppix, pPriv);
> +			}
> +		} else {
> +			ret = NVPutBlitImage(pScrn, pPriv->video_mem, offset, id,
> +					     dstPitch, &dstBox, 0, 0, xb, yb, npixels,
> +					     nlines, src_w, src_h, drw_w, drw_h,
> +					     clipBoxes, ppix);
>  		}
>  
>  		if (ret != Success)
>  			return ret;
> -	} else {
> -		ret = NVPutBlitImage(pScrn, pPriv->video_mem, offset, id,
> -				     dstPitch, &dstBox, 0, 0, xb, yb, npixels,
> -				     nlines, src_w, src_h, drw_w, drw_h,
> -				     clipBoxes, ppix);
> -		if (ret != Success)
> -			return ret;
> -	}
>  
>  #ifdef COMPOSITE
> -	/* Damage tracking */
> -	if (!(action_flags & USE_OVERLAY))
> +		/* Damage tracking */
>  		DamageDamageRegion(&ppix->drawable, clipBoxes);
>  #endif
> +	}
>  
>  	return Success;
>  }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/nouveau/attachments/20100730/3405aba8/attachment.pgp>