On Thu, Mar 18, 2021 at 06:27:37AM +0100, Greg KH wrote: > On Thu, Mar 18, 2021 at 10:14:30AM +0800, jiangzhipeng wrote: > > On Wed, 17 Mar 2021 15:51:02 +0100 > > Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > On Wed, Mar 17, 2021 at 10:25:13PM +0800, jzp0409 wrote: > > > > From: "edison.jiang" <jiangzhipeng@xxxxxxxxxx> > > > > > > > > Fix undefined behaviour in the usb apci driver by using 'BIT' > > > > marcro. > > > > > > What is undefined about it? > > > > > > > > > > > Signed-off-by: edison.jiang <jiangzhipeng@xxxxxxxxxx> > > > > > > This name is not a valid signed-off-by name (I doubt you sign > > > documents with a ".", right?) and it does not match the "From:" name > > > either, so something needs to be fixed before this patch could be > > > accepted. > > > > > > > --- > > > > drivers/usb/core/usb-acpi.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/usb/core/usb-acpi.c > > > > b/drivers/usb/core/usb-acpi.c index 50b2fc7..3e467a8 100644 > > > > --- a/drivers/usb/core/usb-acpi.c > > > > +++ b/drivers/usb/core/usb-acpi.c > > > > @@ -122,7 +122,7 @@ static enum usb_port_connect_type > > > > usb_acpi_get_connect_type(acpi_handle handle, > > > > * Private to usb-acpi, all the core needs to know is that > > > > * port_dev->location is non-zero when it has been set by the > > > > firmware. */ > > > > -#define USB_ACPI_LOCATION_VALID (1 << 31) > > > > +#define USB_ACPI_LOCATION_VALID BIT(31) > > > > > > I do not understand what this is trying to fix, please be more > > > specific. > > > > > > thanks, > > > > > > greg k-h > > > > cppcheck error: > > linux/drivers/usb/core/usb-acpi.c:191]: (error) Shifting signed 32-bit > > value by 31 bits is undefined behaviour > > > > That does not explain anything, sorry. In other words, cppcheck is usually horrid and you need to do some real work to determine what it is trying to say, and to see if it just is wrong or not. Please do that research and if there is a real issue, submit a patch to resolve it and put that information in the changelog text to explain what is happening. As it is, this change doesn't seem to actually do anything except quiet a random tool. And that tool is wrong here. thanks, greg k-h
