| Message ID | 20200620033007.1444705-9-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Remove uninitialized_var() macro | expand |
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 <broonie@kernel.org>
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 <broonie@kernel.org> Thanks!
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.
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. :)
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/<cover_letter_message_id>
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;
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-glider@google.com/ [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/ [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/ [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/ Fixes: 048177ce3b39 ("spi: spi-davinci: convert to DMA engine API") Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/spi/spi-davinci.c | 1 - 1 file changed, 1 deletion(-)