From: Atari Ace [mailto:atari_ace@verizon.net]

Subject: [patch] Eliminate assignments in conditionals

Hi mamedev,

Assignments in conditionals are never really needed in C and are
occasionally just plain bugs (== gets typed as =).  As such, it would
be good to remove these from MAME so that compilers that warn on this
construct can flag the likely bugs (MSVC does this for example).  The
attached patch does just that.  In addition, it refactors some
repeated code which had this in taito_f3.c into a couple macros. Using
inline functions would unfortunately have required more significant
changes, perhaps I'll tackle that another day.

~aa

src/emu/cpu/alph8201/8201dasm.c

@@ -201,15 +201,15 @@ static void InitDasm8201(void)
 		pdown = 0;
 		type = 0;
 		bit = 7;
-		while ( (chr=*p) && bit >= 0) {
-			p++;
+		while (*p && bit >= 0) {
+			chr = *p++;
 			switch (chr) {
 				case '1': bits |= 1<<bit;
 				case '0': mask |= 1<<bit; bit--; break;
-/*
-                case 'b':
-                    type |= 0x80;
-*/
+#if 0
+				case 'b':
+					type |= 0x80;
+#endif
 				case 'a':
 					pmask |= 1<<bit;
 					pdown  = bit;

src/lib/util/sha1.c

@@ -292,7 +292,8 @@ sha1_update(struct sha1_ctx *ctx,
       buffer += SHA1_DATA_SIZE;
       length -= SHA1_DATA_SIZE;
     }
-  if ((ctx->index = length))     /* This assignment is intended */
+  ctx->index = length;
+  if (length)
     /* Buffer leftovers */
     memcpy(ctx->block, buffer, length);
 }

src/mame/drivers/chqflag.c

@@ -77,12 +77,11 @@ static WRITE8_HANDLER( chqflag_vreg_w )
 	coin_counter_w(0,data & 0x02);
 
 	/* bit 4 = enable rom reading thru K051316 #1 & #2 */
-	if ((K051316_readroms = (data & 0x10))){
+	K051316_readroms = (data & 0x10);
+	if (K051316_readroms)
 		memory_install_read8_handler(machine, 0, ADDRESS_SPACE_PROGRAM, 0x2800, 0x2fff, 0, 0, K051316_rom_1_r);	/* 051316 (ROM test) */
-	}
-	else{
+	else
 		memory_install_read8_handler(machine, 0, ADDRESS_SPACE_PROGRAM, 0x2800, 0x2fff, 0, 0, K051316_1_r);		/* 051316 */
-	}
 
 	/* Bits 3-7 probably control palette dimming in a similar way to TMNT2/Saunset Riders, */
 	/* however I don't have enough evidence to determine the exact behaviour. */

src/mame/machine/konamigx.c

@@ -536,7 +536,8 @@ INLINE void zdrawgfxzoom32GP( bitmap_t *bitmap, const gfx_element *gfx, const re
 
 	// cull off-screen objects
 	if (dst_x > dst_maxx || dst_y > dst_maxy) return;
-	if ((nozoom = (scalex == 0x10000 && scaley == 0x10000)))
+	nozoom = (scalex == 0x10000 && scaley == 0x10000);
+	if (nozoom)
 	{
 		dst_h = dst_w = 16;
 		src_fdy = src_fdx = 1;
@@ -1246,14 +1247,16 @@ void konamigx_mixer(running_machine *machine, bitmap_t *bitmap, const rectangle
 
 
 	// abort if object database failed to initialize
-	if (!(objpool = gx_objpool)) return;
+	objpool = gx_objpool;
+	if (!objpool) return;
 
 	// clear screen with backcolor and update flicker pulse
 	K054338_fill_backcolor(machine, bitmap, konamigx_wrport1_0 & 0x20);
 	parity ^= 1;
 
 	// abort if video has been disabled
-	if (!(disp = K055555_read_register(K55_INPUT_ENABLES))) return;
+	disp = K055555_read_register(K55_INPUT_ENABLES);
+	if (!disp) return;
 	cltc_shdpri = K054338_read_register(K338_REG_CONTROL);
 	if (!(cltc_shdpri & K338_CTL_KILL)) return;
 
@@ -1437,7 +1440,8 @@ void konamigx_mixer(running_machine *machine, bitmap_t *bitmap, const rectangle
 		}
 		else
 		{
-			if ((shadow = k>>10 & 3)) // object has shadow?
+			shadow = k>>10 & 3;
+			if (shadow) // object has shadow?
 			{
 				if (shadow != 1 || K053246_objset1 & 0x20)
 				{
@@ -1670,7 +1674,8 @@ void konamigx_mixer(running_machine *machine, bitmap_t *bitmap, const rectangle
 		alpha = 255;
 		if (drawmode & 2)
 		{
-			if ((alpha = color>>K055555_MIXSHIFT & 3)) alpha = K054338_set_alpha_level(alpha);
+			alpha = color>>K055555_MIXSHIFT & 3;
+			if (alpha) alpha = K054338_set_alpha_level(alpha);
 			if (alpha <= 0) continue;
 		}
 		color &= K055555_COLORMASK;
@@ -2219,7 +2224,9 @@ if((data1=obj[0])&0x80000000)\
 		srcend = src + count * 0x30;
 		do
 		{
-			if (!src[0] || !(i = src[7] & 0xf)) continue; // reject retired or zero-element groups
+			if (!src[0]) continue;
+			i = src[7] & 0xf;
+			if (!i) continue; // reject retired or zero-element groups
 			i <<= 2;
 			hoffs = src[5]>>16;
 			voffs = src[6]>>16;

src/mame/video/bwing.c

@@ -218,7 +218,8 @@ VIDEO_START( bwing )
 	fgfx = machine->gfx[2];
 	bgfx = machine->gfx[3];
 
-	if ((dwptr = fgfx->pen_usage))
+	dwptr = fgfx->pen_usage;
+	if (dwptr)
 	{
 		dwptr[0] = 0;
 		for(i=1; i<BW_NTILES; i++) dwptr[i] = -1;

src/mame/video/konamiic.c

@@ -6437,7 +6437,8 @@ static int K056832_update_linemap(running_machine *machine, bitmap_t *bitmap, in
 #define LINE_WIDTH 512
 
 #define DRAW_PIX(N) \
-	if ((pen = src_ptr[N])) \
+	pen = src_ptr[N]; \
+	if (pen) \
 	{ pen += basepen; xpr_ptr[count+N] = TILEMAP_PIXEL_LAYER0; dst_ptr[count+N] = pen; } else \
 	{ xpr_ptr[count+N] = 0; }
 
@@ -6578,7 +6579,8 @@ void K056832_tilemap_draw(running_machine *machine, bitmap_t *bitmap, const rect
 	cmaxy = cliprect->max_y;
 
 	// flip correction registers
-	if ((flipy = K056832_regs[0] & 0x20))
+	flipy = K056832_regs[0] & 0x20;
+	if (flipy)
 	{
 		corr = K056832_regs[0x3c/2];
 		if (corr & 0x400)
@@ -6587,7 +6589,8 @@ void K056832_tilemap_draw(running_machine *machine, bitmap_t *bitmap, const rect
 	dy += corr;
 	ay = (unsigned)(dy - K056832_LayerOffset[layer][1]) % height;
 
-	if ((flipx = K056832_regs[0] & 0x10))
+	flipx = K056832_regs[0] & 0x10;
+	if (flipx)
 	{
 		corr = K056832_regs[0x3a/2];
 		if (corr & 0x800)
@@ -6855,7 +6858,8 @@ void K056832_tilemap_draw_dj(running_machine *machine, bitmap_t *bitmap, const r
 	cmaxy = cliprect->max_y;
 
 	// flip correction registers
-	if ((flipy = K056832_regs[0] & 0x20))
+	flipy = K056832_regs[0] & 0x20;
+	if (flipy)
 	{
 		corr = K056832_regs[0x3c/2];
 		if (corr & 0x400)
@@ -6864,7 +6868,8 @@ void K056832_tilemap_draw_dj(running_machine *machine, bitmap_t *bitmap, const r
 	dy += corr;
 	ay = (unsigned)(dy - K056832_LayerOffset[layer][1]) % height;
 
-	if ((flipx = K056832_regs[0] & 0x10))
+	flipx = K056832_regs[0] & 0x10;
+	if (flipx)
 	{
 		corr = K056832_regs[0x3a/2];
 		if (corr & 0x800)

src/mame/video/taito_f3.c

@@ -1357,6 +1357,31 @@ static void init_alpha_blend_func(void)
 	} \
 }
 
+#define UPDATE_PIXMAP_SP(pf_num) \
+	if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs)) \
+	{ \
+		sprite_pri=sprite[pf_num]&pval; \
+		if(sprite_pri) \
+		{ \
+			if(sprite[pf_num]&0x100) break; \
+			if(!dpix_sp[sprite_pri]) \
+			{ \
+				if(!(pval&0xf0)) break; \
+				else {dpix_1_sprite(*dsti);*dsti=dval;break;} \
+			} \
+			if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;} \
+		} \
+	}
+
+#define UPDATE_PIXMAP_LP(pf_num) \
+	if (cx>=clip_al##pf_num && cx<clip_ar##pf_num && !(cx>=clip_bl##pf_num && cx<clip_br##pf_num)) \
+	{ \
+		tval=*tsrc##pf_num; \
+		if(tval&0xf0) \
+			if(dpix_lp[pf_num][pval>>4](clut[*src##pf_num])) {*dsti=dval;break;} \
+	}
+
+
 /*============================================================================*/
 
 INLINE void draw_scanlines(running_machine *machine,
@@ -1372,13 +1397,6 @@ INLINE void draw_scanlines(running_machine *machine,
 
 	const int x=46;
 
-	UINT32 sprite_noalp_0=sprite[0]&0x100;
-	UINT32 sprite_noalp_1=sprite[1]&0x100;
-	UINT32 sprite_noalp_2=sprite[2]&0x100;
-	UINT32 sprite_noalp_3=sprite[3]&0x100;
-	UINT32 sprite_noalp_4=sprite[4]&0x100;
-	UINT32 sprite_noalp_5=sprite[5]&0x100;
-
 	static UINT16 *src0=0,*src_s0=0,*src_e0=0,clip_al0=0,clip_ar0=0,clip_bl0=0,clip_br0=0;
 	static UINT8 *tsrc0=0,*tsrc_s0=0;
 	static UINT32 x_count0=0,x_zoom0=0;
@@ -1457,80 +1475,12 @@ INLINE void draw_scanlines(running_machine *machine,
 					UINT8 sprite_pri;
 					switch(skip_layer_num)
 					{
-						case 0: if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs) && (sprite_pri=sprite[0]&pval))
-								{
-									if(sprite_noalp_0) break;
-									if(!dpix_sp[sprite_pri]) break;
-									if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;}
-								}
-								if (cx>=clip_al0 && cx<clip_ar0 && !(cx>=clip_bl0 && cx<clip_br0)) {tval=*tsrc0;if(tval&0xf0) if(dpix_lp[0][pval>>4](clut[*src0])) {*dsti=dval;break;}}
-						case 1: if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs) && (sprite_pri=sprite[1]&pval))
-								{
-									if(sprite_noalp_1) break;
-									if(!dpix_sp[sprite_pri])
-									{
-										if(!(pval&0xf0)) break;
-										else {dpix_1_sprite(*dsti);*dsti=dval;break;}
-									}
-									if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;}
-								}
-								if (cx>=clip_al1 && cx<clip_ar1 && !(cx>=clip_bl1 && cx<clip_br1)) {tval=*tsrc1;if(tval&0xf0) if(dpix_lp[1][pval>>4](clut[*src1])) {*dsti=dval;break;}}
-						case 2: if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs) && (sprite_pri=sprite[2]&pval))
-								{
-									if(sprite_noalp_2) break;
-									if(!dpix_sp[sprite_pri])
-									{
-										if(!(pval&0xf0)) break;
-										else {dpix_1_sprite(*dsti);*dsti=dval;break;}
-									}
-									if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;}
-								}
-								if (cx>=clip_al2 && cx<clip_ar2 && !(cx>=clip_bl2 && cx<clip_br2)) {tval=*tsrc2;if(tval&0xf0) if(dpix_lp[2][pval>>4](clut[*src2])) {*dsti=dval;break;}}
-						case 3: if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs) && (sprite_pri=sprite[3]&pval))
-								{
-									if(sprite_noalp_3) break;
-									if(!dpix_sp[sprite_pri])
-									{
-										if(!(pval&0xf0)) break;
-										else {dpix_1_sprite(*dsti);*dsti=dval;break;}
-									}
-									if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;}
-								}
-								if (cx>=clip_al3 && cx<clip_ar3 && !(cx>=clip_bl3 && cx<clip_br3)) {tval=*tsrc3;if(tval&0xf0) if(dpix_lp[3][pval>>4](clut[*src3])) {*dsti=dval;break;}}
-
-						case 4: if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs) && (sprite_pri=sprite[4]&pval))
-								{
-									if(sprite_noalp_4) break;
-									if(!dpix_sp[sprite_pri])
-									{
-										if(!(pval&0xf0)) break;
-										else {dpix_1_sprite(*dsti);*dsti=dval;break;}
-									}
-									if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;}
-								}
-								if (cx>=clip_al4 && cx<clip_ar4 && !(cx>=clip_bl4 && cx<clip_br4)) {
-									tval=*tsrc4;
-									if(tval&0xf0)
-									{
-										if(dpix_lp[4][pval>>4](clut[*src4]))
-										{
-											*dsti=dval;
-											break;
-										}
-									}
-								}
-
-
-						case 5: if(cx>=clip_als && cx<clip_ars && !(cx>=clip_bls && cx<clip_brs) && (sprite_pri=sprite[5]&pval))
-								{
-									if(sprite_noalp_5) break;
-									if(!dpix_sp[sprite_pri])
-									{
-										if(!(pval&0xf0)) break;
-										else {dpix_1_sprite(*dsti);*dsti=dval;break;}
-									}
-									if(dpix_sp[sprite_pri][pval>>4](*dsti)) {*dsti=dval;break;}
-								}
+						case 0: UPDATE_PIXMAP_SP(0) UPDATE_PIXMAP_LP(0)
+						case 1: UPDATE_PIXMAP_SP(1) UPDATE_PIXMAP_LP(1)
+						case 2: UPDATE_PIXMAP_SP(2) UPDATE_PIXMAP_LP(2)
+						case 3: UPDATE_PIXMAP_SP(3) UPDATE_PIXMAP_LP(3)
+						case 4: UPDATE_PIXMAP_SP(4) UPDATE_PIXMAP_LP(4)
+						case 5: UPDATE_PIXMAP_SP(5)
 								if(!bgcolor) {if(!(pval&0xf0)) {*dsti=0;break;}}
 								else dpix_bg(bgcolor);
 								*dsti=dval;
@@ -1588,7 +1538,8 @@ static void visible_tile_check(running_machine *machine,
 	int opaque_all;
 	int total_elements;
 
-	if(!(alpha_mode=line_t->alpha_mode[line])) return;
+	alpha_mode=line_t->alpha_mode[line];
+	if(!alpha_mode) return;
 
 	total_elements=machine->gfx[1]->total_elements;