Re: [PATCH v2 08/16] spi: davinci: Remove uninitialized_var() usage

2020-07-02 Thread Joe Perches
On Thu, 2020-07-02 at 08:42 -0700, Kees Cook wrote:
> On Thu, Jul 02, 2020 at 04:23:35PM +0100, Mark Brown wrote:
> > On Thu, Jul 02, 2020 at 08:21:40AM -0700, Kees Cook wrote:
> > > On Wed, Jul 01, 2020 at 09:39:20PM +0100, Mark Brown wrote:
> > > > Please copy maintainers on patches :(
> > > Hi! Sorry about that; the CC list was giant, so I had opted for using
> > > subsystem mailing lists where possible.
> > 
> > If you're going to err in a direction there I'd err in the direction of
> > CCing the people not the list - I only saw this since I was looking for
> > something else, I don't normally see stuff in the mailing list folder.
> 
> Yeah, I've gotten conflicting feedback on treewide changes:
> - please CC me on only the one patch, I don't want to see everything else
> - please CC me on the whole series, I want the full context for the change
> 
> I opted toward "CC me on this series", but then I get stuck when the CC
> is giant. I think I may switch back to individual CCs for specific
> patches, and point people to lore if they want greater context. (lore
> didn't exist before...)

IMO:

For a patch series that spans multiple subsystems,
each patch should always CC any specific subsystem
maintainers..

A good trick would be to use the cover letter
message-id: and have each individual patch in the
series reference the cover letter id below the
--- line so any reviewer doesn't have to find the
in-reply-to: message id and then reference the
lore link.

Something like:

---

For complete series see: https://lore.kernel.org/r/




Re: [PATCH v2 08/16] spi: davinci: Remove uninitialized_var() usage

2020-07-02 Thread Kees Cook
On Thu, Jul 02, 2020 at 04:23:35PM +0100, Mark Brown wrote:
> On Thu, Jul 02, 2020 at 08:21:40AM -0700, Kees Cook wrote:
> > On Wed, Jul 01, 2020 at 09:39:20PM +0100, Mark Brown wrote:
> 
> > > Please copy maintainers on patches :(
> 
> > Hi! Sorry about that; the CC list was giant, so I had opted for using
> > subsystem mailing lists where possible.
> 
> If you're going to err in a direction there I'd err in the direction of
> CCing the people not the list - I only saw this since I was looking for
> something else, I don't normally see stuff in the mailing list folder.

Yeah, I've gotten conflicting feedback on treewide changes:
- please CC me on only the one patch, I don't want to see everything else
- please CC me on the whole series, I want the full context for the change

I opted toward "CC me on this series", but then I get stuck when the CC
is giant. I think I may switch back to individual CCs for specific
patches, and point people to lore if they want greater context. (lore
didn't exist before...)

Thanks for the poke to make me reconsider this workflow. :)

-- 
Kees Cook


Re: [PATCH v2 08/16] spi: davinci: Remove uninitialized_var() usage

2020-07-02 Thread Mark Brown
On Thu, Jul 02, 2020 at 08:21:40AM -0700, Kees Cook wrote:
> On Wed, Jul 01, 2020 at 09:39:20PM +0100, Mark Brown wrote:

> > Please copy maintainers on patches :(

> Hi! Sorry about that; the CC list was giant, so I had opted for using
> subsystem mailing lists where possible.

If you're going to err in a direction there I'd err in the direction of
CCing the people not the list - I only saw this since I was looking for
something else, I don't normally see stuff in the mailing list folder.


signature.asc
Description: PGP signature


Re: [PATCH v2 08/16] spi: davinci: Remove uninitialized_var() usage

2020-07-02 Thread Kees Cook
On Wed, Jul 01, 2020 at 09:39:20PM +0100, Mark Brown wrote:
> On Fri, Jun 19, 2020 at 08:29:59PM -0700, Kees Cook wrote:
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > "unused variable"). If the compiler thinks it is uninitialized, either
> > simply initialize the variable or make compiler changes. As a precursor
> > to removing[2] this[3] macro[4], just remove this variable since it was
> > actually unused:
> 
> Please copy maintainers on patches :(

Hi! Sorry about that; the CC list was giant, so I had opted for using
subsystem mailing lists where possible.

> Acked-by: Mark Brown 

Thanks!

-- 
Kees Cook


Re: [PATCH v2 08/16] spi: davinci: Remove uninitialized_var() usage

2020-07-01 Thread Mark Brown
On Fri, Jun 19, 2020 at 08:29:59PM -0700, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just remove this variable since it was
> actually unused:

Please copy maintainers on patches :(

Acked-by: Mark Brown 


signature.asc
Description: PGP signature


[PATCH v2 08/16] spi: davinci: Remove uninitialized_var() usage

2020-06-19 Thread Kees Cook
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just remove this variable since it was
actually unused:

drivers/spi/spi-davinci.c: In function ‘davinci_spi_bufs’:
drivers/spi/spi-davinci.c:579:11: warning: unused variable ‘rx_buf_count’ 
[-Wunused-variable]
  579 |  unsigned rx_buf_count;
  |   ^~~~

[1] https://lore.kernel.org/lkml/20200603174714.192027-1-gli...@google.com/
[2] 
https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1tgqcr5vqkczwj0qxk6cernou6eedsuda...@mail.gmail.com/
[3] 
https://lore.kernel.org/lkml/ca+55afwgbgqhbp1fkxvrkepzyr5j8n1vkt1vzdz9knmpuxh...@mail.gmail.com/
[4] 
https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yvju65tplgn_ybynv0ve...@mail.gmail.com/

Fixes: 048177ce3b39 ("spi: spi-davinci: convert to DMA engine API")
Reviewed-by: Nick Desaulniers 
Signed-off-by: Kees Cook 
---
 drivers/spi/spi-davinci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index f71c497393a6..f50c0c79cbdf 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -576,7 +576,6 @@ static int davinci_spi_bufs(struct spi_device *spi, struct 
spi_transfer *t)
u32 errors = 0;
struct davinci_spi_config *spicfg;
struct davinci_spi_platform_data *pdata;
-   unsigned uninitialized_var(rx_buf_count);
 
dspi = spi_master_get_devdata(spi->master);
pdata = &dspi->pdata;
-- 
2.25.1