Hi,
On Mon, Feb 28, 2022 at 10:36:59PM +, Limonciello, Mario wrote:
> [AMD Official Use Only]
>
> > -Original Message-
> > From: Lukas Wunner
> > Sent: Monday, February 28, 2022 16:33
> > To: Bjorn Helgaas
> > Cc: Limonciello, Mario ; Mika Westerberg
> > ; Michael Jamet
> > ; open
On Mon, Feb 28, 2022 at 12:14:44PM -0800, Linus Torvalds wrote:
> On Mon, Feb 28, 2022 at 12:10 PM Linus Torvalds
> wrote:
> >
> > We can do
> >
> > typeof(pos) pos
> >
> > in the 'for ()' loop, and never use __iter at all.
> >
> > That means that inside the for-loop, we use a _different_
On Mon, Feb 28, 2022 at 3:45 PM James Bottomley
wrote:
> ...
> > Just from skimming over the patches to change this and experience
> > with the drivers/subsystems I help to maintain I think the primary
> > pattern looks something like this:
> >
> > list_for_each_entry(entry, head, member) {
> >
On Mon, Feb 28, 2022 at 12:16 PM Matthew Wilcox wrote:
>
> Then we can never use -Wshadow ;-( I'd love to be able to turn it on;
> it catches real bugs.
Oh, we already can never use -Wshadow regardless of things like this.
That bridge hasn't just been burned, it never existed in the first
This is the first patch removing several categories of use cases of
the list iterator variable past the loop.
This is follow up to the discussion in:
https://lore.kernel.org/all/20220217184829.1991035-1-jakobkosc...@gmail.com/
As concluded in:
On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel wrote:
>
> The goal of this is to get compiler warnings right? This would indeed be
> great.
Yes, so I don't mind having a one-time patch that has been gathered
using some automated checker tool, but I don't think that works from a
long-term
> On 28. Feb 2022, at 12:20, Greg KH wrote:
>
> On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote:
>> If the list does not contain the expected element, the value of
>> list_for_each_entry() iterator will not point to a valid structure.
>> To avoid type confusion in such case, the
On Mon, Feb 28, 2022 at 3:26 PM Matthew Wilcox wrote:
>
> #define ___PASTE(a, b) a##b
> #define __PASTE(a, b) ___PASTE(a, b)
> #define _min(a, b, u) ({ \
Yeah, except that's ugly beyond belief, plus it's literally not what
we do in the kernel.
Really. The "-Wshadow doesn't work on the
On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote:
> diff --git a/drivers/scsi/scsi_transport_sas.c
> b/drivers/scsi/scsi_transport_sas.c
> index 4ee578b181da..a8cbd90db9d2 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -1060,26
On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote:
> If the list does not contain the expected element, the value of
> list_for_each_entry() iterator will not point to a valid structure.
> To avoid type confusion in such case, the list iterator
> scope will be limited to
This is a bit more work (and a lot more noise), but I'd prefer if
this were split into as many patches as there are components.
I'm not going to review the parts of the patches that don't concern me,
and if something turns out to be a problem later one (it shouldn't but
one never knows) it'll be
On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
> Am 28.02.22 um 20:56 schrieb Linus Torvalds:
> > On Mon, Feb 28, 2022 at 4:19 AM Christian König
> > wrote:
> > > I don't think that using the extra variable makes the code in any
> > > way
> > > more reliable or easier to read.
> > So I
On Mon, 2022-02-28 at 23:59 +0200, Mike Rapoport wrote:
>
> On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley <
> james.bottom...@hansenpartnership.com> wrote:
> > On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
[...]
> > > > I do wish we could actually poison the 'pos' value
From: Matthew Wilcox
> Sent: 28 February 2022 20:16
>
> On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote:
> > We can do
> >
> > typeof(pos) pos
> >
> > in the 'for ()' loop, and never use __iter at all.
> >
> > That means that inside the for-loop, we use a _different_ 'pos'
On Mon, Feb 28, 2022 at 05:28:58PM -0500, James Bottomley wrote:
> On Mon, 2022-02-28 at 23:59 +0200, Mike Rapoport wrote:
> >
> > On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley <
> > james.bottom...@hansenpartnership.com> wrote:
> > > On Mon, 2022-02-28 at 21:07 +0100, Christian
On Mon, 21 Feb 2022 10:59:09 +0100, Maxime Ripard wrote:
> The nouveau KMS driver will call drm_plane_create_zpos_property() with
> an init value depending on the plane purpose.
>
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in nv50_wndw_reset().
> On 28. Feb 2022, at 21:10, Linus Torvalds
> wrote:
>
> On Mon, Feb 28, 2022 at 12:03 PM Linus Torvalds
> wrote:
>>
>> Side note: we do need *some* way to do it.
>
> Ooh.
>
> This patch is a work of art.
>
> And I mean that in the worst possible way.
>
> We can do
>
>
On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley
wrote:
>On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
>> Am 28.02.22 um 20:56 schrieb Linus Torvalds:
>> > On Mon, Feb 28, 2022 at 4:19 AM Christian König
>> > wrote:
>> > > I don't think that using the extra variable
On Mon, 2022-02-28 at 20:16 +, Matthew Wilcox wrote:
> On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote:
> > We can do
> >
> > typeof(pos) pos
> >
> > in the 'for ()' loop, and never use __iter at all.
> >
> > That means that inside the for-loop, we use a _different_
On Mon, Feb 28, 2022 at 12:37:15PM -0800, Linus Torvalds wrote:
> On Mon, Feb 28, 2022 at 12:16 PM Matthew Wilcox wrote:
> >
> > Then we can never use -Wshadow ;-( I'd love to be able to turn it on;
> > it catches real bugs.
>
> Oh, we already can never use -Wshadow regardless of things like
On Mon, Feb 28, 2022 at 4:45 PM Linus Torvalds
wrote:
>
> Yeah, except that's ugly beyond belief, plus it's literally not what
> we do in the kernel.
(Of course, I probably shouldn't have used 'min()' as an example,
because that is actually one of the few places where we do exactly
that, using
On Mon, 2022-02-28 at 14:24 +0300, Dan Carpenter wrote:
> a multi-line indent gets curly braces for readability even though
> it's not required by C. And then both sides would get curly braces.
That's more your personal preference than a coding style guideline.
On Mon, Feb 28, 2022 at 12:08:22PM +0100, Jakob Koschel wrote:
> diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c
> b/drivers/infiniband/hw/hfi1/tid_rdma.c
> index 2a7abf7a1f7f..a069847b56aa 100644
> --- a/drivers/infiniband/hw/hfi1/tid_rdma.c
> +++ b/drivers/infiniband/hw/hfi1/tid_rdma.c
> @@
On Mon, Feb 28, 2022 at 4:19 AM Christian König
wrote:
>
> I don't think that using the extra variable makes the code in any way
> more reliable or easier to read.
So I think the next step is to do the attached patch (which requires
that "-std=gnu11" that was discussed in the original thread).
On Mon, Feb 28, 2022 at 12:03 PM Linus Torvalds
wrote:
>
> Side note: we do need *some* way to do it.
Ooh.
This patch is a work of art.
And I mean that in the worst possible way.
We can do
typeof(pos) pos
in the 'for ()' loop, and never use __iter at all.
That means that inside the
On Mon, Feb 28, 2022 at 4:38 PM Segher Boessenkool
wrote:
>
> In C its scope is the rest of the declaration and the entire loop, not
> anything after it. This was the same in C++98 already, btw (but in
> pre-standard versions of C++ things were like you remember, yes, and it
> was painful).
When list_for_each_entry() completes the iteration over the whole list
without breaking the loop, the iterator value will be a bogus pointer
computed based on the head element.
While it is safe to use the pointer to determine if it was computed
based on the head element, either with
> On 28. Feb 2022, at 12:24, Dan Carpenter wrote:
>
> On Mon, Feb 28, 2022 at 12:08:17PM +0100, Jakob Koschel wrote:
>> diff --git a/drivers/usb/gadget/udc/at91_udc.c
>> b/drivers/usb/gadget/udc/at91_udc.c
>> index 9040a0561466..0fd0307bc07b 100644
>> --- a/drivers/usb/gadget/udc/at91_udc.c
From: Linus Torvalds
> Sent: 28 February 2022 19:56
> On Mon, Feb 28, 2022 at 4:19 AM Christian König
> wrote:
> >
> > I don't think that using the extra variable makes the code in any way
> > more reliable or easier to read.
>
> So I think the next step is to do the attached patch (which
If the list does not contain the expected element, the value of
list_for_each_entry() iterator will not point to a valid structure.
To avoid type confusion in such case, the list iterator
scope will be limited to list_for_each_entry() loop.
In preparation to limiting scope of a list iterator to
On Mon, 2022-02-28 at 21:56 +0100, Christian König wrote:
>
> Am 28.02.22 um 21:42 schrieb James Bottomley:
> > On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
> > > Am 28.02.22 um 20:56 schrieb Linus Torvalds:
> > > > On Mon, Feb 28, 2022 at 4:19 AM Christian König
> > > > wrote:
> >
On Mon, Feb 28, 2022 at 12:29 PM Johannes Berg
wrote:
>
> If we're willing to change the API for the macro, we could do
>
> list_for_each_entry(type, pos, head, member)
>
> and then actually take advantage of -Wshadow?
See my reply to Willy. There is no way -Wshadow will ever happen.
I
On Mon, Feb 28, 2022 at 12:08:19PM +0100, Jakob Koschel wrote:
> The list iterator value will *always* be set by list_for_each_entry().
> It is incorrect to assume that the iterator value will be NULL if the
> list is empty.
>
> Instead of checking the pointer it should be checked if
> the list
Hi,
On Mon, Feb 21, 2022 at 05:42:36PM +0100, Karol Herbst wrote:
> On Mon, Feb 21, 2022 at 11:00 AM Maxime Ripard wrote:
> >
> > The nouveau KMS driver will call drm_plane_create_zpos_property() with
> > an init value depending on the plane purpose.
> >
> > Since the initial value wasn't
> On 28. Feb 2022, at 21:56, Christian König wrote:
>
>
>
> Am 28.02.22 um 21:42 schrieb James Bottomley:
>> On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
>>> Am 28.02.22 um 20:56 schrieb Linus Torvalds:
On Mon, Feb 28, 2022 at 4:19 AM Christian König
wrote:
If the list representing the request queue does not contain the expected
request, the value of list_for_each_entry() iterator will not point to a
valid structure. To avoid type confusion in such case, the list iterator
scope will be limited to list_for_each_entry() loop.
In preparation to
On Mon, Feb 28, 2022 at 01:03:36PM +0100, Jakob Koschel wrote:
> >> @@ -954,7 +957,6 @@ net2272_dequeue(struct usb_ep *_ep, struct usb_request
> >> *_req)
> >>dev_dbg(ep->dev->dev, "unlink (%s) pio\n", _ep->name);
> >>net2272_done(ep, req, -ECONNRESET);
> >>}
> >> -
On Mon, Feb 28, 2022 at 12:10 PM Linus Torvalds
wrote:
>
> We can do
>
> typeof(pos) pos
>
> in the 'for ()' loop, and never use __iter at all.
>
> That means that inside the for-loop, we use a _different_ 'pos' than outside.
The thing that makes me throw up in my mouth a bit is that in
The list iterator value will *always* be set by list_for_each_entry().
It is incorrect to assume that the iterator value will be NULL if the
list is empty.
Instead of checking the pointer it should be checked if
the list is empty.
In acpi_get_pmu_hw_inf() instead of setting the pointer to NULL
on
Fix following coccicheck warning:
drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c:316:11-12:
WARNING this kind of initialization is deprecated.
`void *map = map` has the same form of
uninitialized_var() macro. I remove the redundant assignement. It has
been tested with gcc (Debian 8.3.0-6)
On Mon, Feb 28, 2022 at 11:56 AM Linus Torvalds
wrote:
>
> I do wish we could actually poison the 'pos' value after the loop
> somehow - but clearly the "might be uninitialized" I was hoping for
> isn't the way to do it.
Side note: we do need *some* way to do it.
Because if we make that change,
On Mon, Feb 28, 2022 at 12:08:17PM +0100, Jakob Koschel wrote:
> diff --git a/drivers/usb/gadget/udc/at91_udc.c
> b/drivers/usb/gadget/udc/at91_udc.c
> index 9040a0561466..0fd0307bc07b 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -150,13 +150,14
On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote:
> We can do
>
> typeof(pos) pos
>
> in the 'for ()' loop, and never use __iter at all.
>
> That means that inside the for-loop, we use a _different_ 'pos' than outside.
Then we can never use -Wshadow ;-( I'd love to be
When list_for_each_entry() completes the iteration over the whole list
without breaking the loop, the iterator value will *always* be a bogus
pointer computed based on the head element.
To avoid type confusion use the actual list head directly instead of last
iterator value.
Signed-off-by: Jakob
The list iterator variable will be a bogus pointer if no break was hit.
Dereferencing it could load *any* out-of-bounds/undefined value
making it unsafe to use that in the comparision to determine if the
specific element was found.
This is fixed by using a separate list iterator variable for the
On 09 2月 22 14:53:19, Cai Huoqing wrote:
> The nouveau driver depends on include/linux/list.h instead of
> nvif/list.h, so remove the obstacle-nvif/list.h.
>
> Signed-off-by: Cai Huoqing
> ---
Ping :)
> drivers/gpu/drm/nouveau/include/nvif/list.h | 353
> 1 file changed,
On Mon, Feb 28, 2022 at 04:13:44PM -0600, Bjorn Helgaas wrote:
> On Mon, Feb 28, 2022 at 03:33:13PM +, Limonciello, Mario wrote:
> > > On Fri, Feb 25, 2022 at 11:42:24AM -0600, Bjorn Helgaas wrote:
> > > > That would just leave the "PCI_VSEC_ID_INTEL_TBT implies external-
> > > facing"
> > > >
On Mon, Feb 28, 2022 at 03:33:13PM +, Limonciello, Mario wrote:
> [AMD Official Use Only]
>
> >
> > On Fri, Feb 25, 2022 at 11:42:24AM -0600, Bjorn Helgaas wrote:
> > > That would just leave the "PCI_VSEC_ID_INTEL_TBT implies external-
> > facing"
> > > assumption above. Not having a
Hi Bjorn,
On Fri, Feb 25, 2022 at 11:42:24AM -0600, Bjorn Helgaas wrote:
> That would just leave the "PCI_VSEC_ID_INTEL_TBT implies external-facing"
> assumption above. Not having a Thunderbolt spec, I have no idea how
> you deal with that.
You can download the spec here:
49 matches
Mail list logo
