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] [day] [month] [year] [list]
Date: Mon, 4 Mar 2024 11:59:13 -0600
From: Alex Elder <elder@...e.org>
To: Dileep Sankhla <dileepsankhla.ds@...il.com>,
 Greg KH <gregkh@...uxfoundation.org>
Cc: greybus-dev@...ts.linaro.org, linux-staging@...ts.linux.dev,
 linux-kernel@...r.kernel.org, pure.logic@...us-software.ie,
 johan@...nel.org, elder@...nel.org
Subject: Re: [PATCH] staging: greybus: put macro in a do - while loop

On 2/25/24 3:49 AM, Dileep Sankhla wrote:
> On Sun, Feb 25, 2024 at 2:26 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>> Did you test build this?
> 
> Hello Greg,
> 
> Yes. No new warning/error was encountered on building the kernel.

Then your build must not have been compiling your changed
code, because the result of your change produces code that
will not compile successfully.

If you look at where gb_loopback_stats_attrs() is called, it's
used only at outer scope, in "drivers/staging/greybus/loopback.c".

Adding do { ... } while() at outer scope is nonsensical.

> 
>>>   #define gb_loopback_attr(field, type)                                        \
>>>   static ssize_t field##_show(struct device *dev,                              \
>>
>> Why did you only change one if you thought this was a valid change?
> 
> 1. As per my C background, I think no other macros in the above source
> code file need to be enclosed in a do - while loop.

gb_loopback_stats_attrs() must *not* be enclosed in a do..while loop.

> 2. I am writing the patch because of the Eudyptula Challenge, and I
> have to fix "one coding style problem" in any of the files in
> drivers/staging/. The above one was one of them.

I support the challenge.  But you need to be sure your fix actually
works, and in particular (in this case) that it compiles correctly.

					-Alex

> 
> Regards,
> Dileep


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ