netdev - Re: [PATCH] net/ehea: bitops work on unsigned longs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1230714172.15389.63.camel@pasglop>
Date:	Wed, 31 Dec 2008 20:02:51 +1100
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	David Miller <davem@...emloft.net>
Cc:	sfr@...b.auug.org.au, tklein@...ibm.com, netdev@...r.kernel.org,
	raisch@...ibm.com, themann@...ibm.com, linuxppc-dev@...abs.org
Subject: Re: [PATCH] net/ehea: bitops work on unsigned longs

On Tue, 2008-12-30 at 21:51 -0800, David Miller wrote:
> From: Stephen Rothwell <sfr@...b.auug.org.au>
> Date: Wed, 31 Dec 2008 14:18:53 +1100
> 
> > These changes will avoid several warnings when we change u64 to unsigned
> > long long.
> > 
> > Also, ehea_driver_flags is only used in ehca_main.c
> > 
> > Signed-off-by: Stephen Rothwell <sfr@...b.auug.org.au>
> > ---
> 
> And also rejected, just like the previous two.
> 
> This is so much worse than the problems we had with
> printing u64's and it's being done as a result to
> the "fix" for that.

Hi David !

I think this patch specifically is different and deserve a second look.

While the other patches are somewhat debatable (I do agree with you for
example that we shouldn't break the possibility of building for 32-bit,
and we shouldn't artifically add crud to silence warnings caused by the
u64 type change etc... I'll send separate replies to the other messages
later), in the case of this specific patch, I think it's actually more
correct to define a "flags" field that is used by set_bit() and
test_bit() using "long" rather than "u64" (for that same reason you
mention, which is 32/64-bit compatibility).

IE. The bitops operate on longs. Thus the field should be a long,
period. The compatibility here consists of making sure we don't use bits
above 31, which in this case is find since we use a short enum.

Thus the patch is makes the code cleaner and more correct regardless of
the type change of u64.

So from my side, this patch is

Acked-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html