netdev - Re: [PATCH try#2] Blackfin ethernet driver: on chip ethernet MAC controller driver

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: <200707151236.51779.mb@bu3sch.de>
Date:	Sun, 15 Jul 2007 12:36:51 +0200
From:	Michael Buesch <mb@...sch.de>
To:	bryan.wu@...log.com
Cc:	Mike Frysinger <vapier.adi@...il.com>,
	Jeff Garzik <jeff@...zik.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH try#2] Blackfin ethernet driver: on chip ethernet MAC controller driver

On Sunday 15 July 2007 11:27:09 Bryan Wu wrote:
> +#if defined(CONFIG_BFIN_MAC_USE_L1)
> +# define bfin_mac_alloc(dma_handle, size)  l1_data_sram_zalloc(size)
> +# define bfin_mac_free(dma_handle, ptr)    l1_data_sram_free(ptr)
> +#else
> +# define bfin_mac_alloc(dma_handle, size) \
> +	 dma_alloc_coherent(NULL, size, dma_handle, GFP_NORMAL)

What is GFP_NORMAL? It's not defined in latest linus' tree.
I think you should use GFP_KERNEL, if you can sleep, or GFP_ATOMIC,
if you can't.

The rest looks OK. Except the endianess issues. It might be no
issue on the hardware this runs on, but in favour of "clean code"
you might use leXX_to_cpu and friends anyway. :)
This kind of bugs is done so often, even in places where it _does_
matter. So, at least for the human reader, the leXX_to_cpu stuff
says that you really understood what you were doing when writing
the code. The current code says (to me), that it works by
accident, somehow, although it seems you knew what you were doing. :)

-- 
Greetings Michael.
-
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