From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Subject: Re: [patch 1/2] ALSA: oxfw: some signedness bugs Date: Sat, 13 Dec 2014 14:35:19 +0300 Message-ID: <20141213113519.GA4911@mwanda> References: <20141212192703.GA22234@mwanda> <548BC973.3090004@sakamocchi.jp> <20141213070414.GA4973@mwanda> <548C1196.4010503@sakamocchi.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <548C1196.4010503@sakamocchi.jp> Sender: kernel-janitors-owner@vger.kernel.org To: Takashi Sakamoto Cc: Takashi Iwai , kernel-janitors@vger.kernel.org, alsa-devel@alsa-project.org, Clemens Ladisch List-Id: alsa-devel@alsa-project.org On Sat, Dec 13, 2014 at 07:14:46PM +0900, Takashi Sakamoto wrote: > On Dec 13 2014 16:04, Dan Carpenter wrote: > > On Sat, Dec 13, 2014 at 02:06:59PM +0900, Takashi Sakamoto wrote: > >> On Dec 13 2014 04:27, Dan Carpenter wrote: > >>> This code tends to use unsigned variables by default and it causes > >>> signedness bugs when we use negative variables for error handling. > >>> The "i" and "j" variables are used to iterated over small positive > >>> values and so they should be type "int". The "len" variable doesn't > >>> *need* to be signed but it should be signed to make the code easier to > >>> read and audit. > >>> > >>> Signed-off-by: Dan Carpenter > >> > >> Thanks, but I prefer to use 'unsigned int' for loop counter, like the > >> other drivers. > > > > Unthinking use of unsigned int every where is a kind of leprosy. It > > just makes the code hard to read and it causes bugs. > > In the beginning of last year, I received a comment from Clemens to use > 'unsigned int' type for loop counter instead of 'int' type. Since then, > for ALSA firewire stack, I've been following his advice. (but actually I > missed to OXFW driver...) > > For consistency inner this stack, I request you to follow this. > That's nonsense advice. How many bugs has it prevented? It causes a lot. Whatever, just fix it and give me the reported-by tag. regards, dan carpenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Sat, 13 Dec 2014 11:35:19 +0000 Subject: Re: [patch 1/2] ALSA: oxfw: some signedness bugs Message-Id: <20141213113519.GA4911@mwanda> List-Id: References: <20141212192703.GA22234@mwanda> <548BC973.3090004@sakamocchi.jp> <20141213070414.GA4973@mwanda> <548C1196.4010503@sakamocchi.jp> In-Reply-To: <548C1196.4010503@sakamocchi.jp> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Takashi Sakamoto Cc: Takashi Iwai , kernel-janitors@vger.kernel.org, alsa-devel@alsa-project.org, Clemens Ladisch On Sat, Dec 13, 2014 at 07:14:46PM +0900, Takashi Sakamoto wrote: > On Dec 13 2014 16:04, Dan Carpenter wrote: > > On Sat, Dec 13, 2014 at 02:06:59PM +0900, Takashi Sakamoto wrote: > >> On Dec 13 2014 04:27, Dan Carpenter wrote: > >>> This code tends to use unsigned variables by default and it causes > >>> signedness bugs when we use negative variables for error handling. > >>> The "i" and "j" variables are used to iterated over small positive > >>> values and so they should be type "int". The "len" variable doesn't > >>> *need* to be signed but it should be signed to make the code easier to > >>> read and audit. > >>> > >>> Signed-off-by: Dan Carpenter > >> > >> Thanks, but I prefer to use 'unsigned int' for loop counter, like the > >> other drivers. > > > > Unthinking use of unsigned int every where is a kind of leprosy. It > > just makes the code hard to read and it causes bugs. > > In the beginning of last year, I received a comment from Clemens to use > 'unsigned int' type for loop counter instead of 'int' type. Since then, > for ALSA firewire stack, I've been following his advice. (but actually I > missed to OXFW driver...) > > For consistency inner this stack, I request you to follow this. > That's nonsense advice. How many bugs has it prevented? It causes a lot. Whatever, just fix it and give me the reported-by tag. regards, dan carpenter