| Message ID | 20231109212157.1454726-1-singhabhinav9051571833@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | driver : edac : Fix warning using plain integer as NULL | expand |
On 11/10/23 02:51, Abhinav Singh wrote: > Sparse static analysis tools generate a warning with this message > "Using plain integer as NULL pointer". In this case this warning is > being shown because we are trying to initialize pointer to NULL using > integer value 0. > > Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com> > --- > drivers/edac/i7core_edac.c | 2 +- > drivers/edac/sb_edac.c | 10 +++++----- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c > index 23d25724bae4..08bf20c60111 100644 > --- a/drivers/edac/i7core_edac.c > +++ b/drivers/edac/i7core_edac.c > @@ -376,7 +376,7 @@ static const struct pci_id_table pci_dev_table[] = { > PCI_ID_TABLE_ENTRY(pci_dev_descr_i7core_nehalem), > PCI_ID_TABLE_ENTRY(pci_dev_descr_lynnfield), > PCI_ID_TABLE_ENTRY(pci_dev_descr_i7core_westmere), > - {0,} /* 0 terminated list. */ > + {NULL,} /* 0 terminated list. */ > }; > > /* > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index 0c779a0326b6..a3f50a66de33 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -439,7 +439,7 @@ static const struct pci_id_descr pci_dev_descr_sbridge[] = { > > static const struct pci_id_table pci_dev_descr_sbridge_table[] = { > PCI_ID_TABLE_ENTRY(pci_dev_descr_sbridge, ARRAY_SIZE(pci_dev_descr_sbridge), 1, SANDY_BRIDGE), > - {0,} /* 0 terminated list. */ > + {NULL,} /* 0 terminated list. */ > }; > > /* This changes depending if 1HA or 2HA: > @@ -505,7 +505,7 @@ static const struct pci_id_descr pci_dev_descr_ibridge[] = { > > static const struct pci_id_table pci_dev_descr_ibridge_table[] = { > PCI_ID_TABLE_ENTRY(pci_dev_descr_ibridge, 12, 2, IVY_BRIDGE), > - {0,} /* 0 terminated list. */ > + {NULL,} /* 0 terminated list. */ > }; > > /* Haswell support */ > @@ -576,7 +576,7 @@ static const struct pci_id_descr pci_dev_descr_haswell[] = { > > static const struct pci_id_table pci_dev_descr_haswell_table[] = { > PCI_ID_TABLE_ENTRY(pci_dev_descr_haswell, 13, 2, HASWELL), > - {0,} /* 0 terminated list. */ > + {NULL,} /* 0 terminated list. */ > }; > > /* Knight's Landing Support */ > @@ -620,7 +620,7 @@ static const struct pci_id_descr pci_dev_descr_knl[] = { > > static const struct pci_id_table pci_dev_descr_knl_table[] = { > PCI_ID_TABLE_ENTRY(pci_dev_descr_knl, ARRAY_SIZE(pci_dev_descr_knl), 1, KNIGHTS_LANDING), > - {0,} > + {NULL,} > }; > > /* > @@ -686,7 +686,7 @@ static const struct pci_id_descr pci_dev_descr_broadwell[] = { > > static const struct pci_id_table pci_dev_descr_broadwell_table[] = { > PCI_ID_TABLE_ENTRY(pci_dev_descr_broadwell, 10, 2, BROADWELL), > - {0,} /* 0 terminated list. */ > + {NULL,} /* 0 terminated list. */ > }; > > Hello maintainers, any reviews or comments Thank You, Abhinav Singh
On Fri, Nov 10, 2023 at 02:51:57AM +0530, Abhinav Singh wrote: > Sparse static analysis tools generate a warning with this message > "Using plain integer as NULL pointer". In this case this warning is > being shown because we are trying to initialize pointer to NULL using > integer value 0. And that is a problem because?
On 11/27/23 22:44, Borislav Petkov wrote: > On Fri, Nov 10, 2023 at 02:51:57AM +0530, Abhinav Singh wrote: >> Sparse static analysis tools generate a warning with this message >> "Using plain integer as NULL pointer". In this case this warning is >> being shown because we are trying to initialize pointer to NULL using >> integer value 0. > > And that is a problem because? > Hello, thanks for reviewing this. As of now this is only a warning issue in kernel. I saw this post by linus https://www.spinics.net/lists/linux-sparse/msg10066.html and thought of submitting a patch. Also a similar patch of mine got accepted https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2560740.html, so thought about opening this one as well. Thank You, Abhinav Singh
On Mon, Nov 27, 2023 at 11:53:02PM +0530, Abhinav Singh wrote: > Hello, thanks for reviewing this. As of now this is only a warning issue in > kernel. I saw this post by linus > https://www.spinics.net/lists/linux-sparse/msg10066.html and thought of > submitting a patch. Also a similar patch of mine got accepted > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2560740.html, > so thought about opening this one as well. Lemme try to understand what you're saying: just because someone accepted a patch of yours, then others should not ask you to improve your commit message so that it explains *why* a change should be done. How about you put the gist of what Linus is saying in your commit message? Don't you think it would be a much better commit message then? Especially if it explains why, even if it is the case that 0 == NULL, we don't want those in the kernel. Hmmm?
On 11/28/23 00:09, Borislav Petkov wrote: > On Mon, Nov 27, 2023 at 11:53:02PM +0530, Abhinav Singh wrote: >> Hello, thanks for reviewing this. As of now this is only a warning issue in >> kernel. I saw this post by linus >> https://www.spinics.net/lists/linux-sparse/msg10066.html and thought of >> submitting a patch. Also a similar patch of mine got accepted >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2560740.html, >> so thought about opening this one as well. > > Lemme try to understand what you're saying: just because someone > accepted a patch of yours, then others should not ask you to improve > your commit message so that it explains *why* a change should be done. > > How about you put the gist of what Linus is saying in your commit > message? Don't you think it would be a much better commit message then? > > Especially if it explains why, even if it is the case that 0 == NULL, we > don't want those in the kernel. > > Hmmm? > Hello, my sincere apologies, I wrongly interpreted that you were asking for a reason in reply rather than in the commit message itself. Yeah I agree putting a reason in the commit message will make more sense. Just to be correct this time, I need to put a reason why this needs to be fixed, in the patch itself, right? Thank You, Abhinav Singh
On Tue, Nov 28, 2023 at 12:23:54AM +0530, Abhinav Singh wrote: > Just to be correct this time, I need to put a reason why this needs to > be fixed, in the patch itself, right? No, the commit message is perfectly fine for that.
On 11/28/23 01:35, Borislav Petkov wrote: > On Tue, Nov 28, 2023 at 12:23:54AM +0530, Abhinav Singh wrote: >> Just to be correct this time, I need to put a reason why this needs to >> be fixed, in the patch itself, right? > > No, the commit message is perfectly fine for that. > Okay, so I guess as of now there is no change needed, so I need not send a v2 patch, right? Thank You, Abhinav Singh
On Tue, Nov 28, 2023 at 01:49:06AM +0530, Abhinav Singh wrote:a > Okay, so I guess as of now there is no change needed, so I need not > send a v2 patch, right? As of now, you should go back and reread the whole thread. I'm going to give you one last example and then stop: Imagine one fine day you're doing git archeology, you find the place in the code about which you want to find out why it was changed the way it is now. You do git annotate <filename> ... find the line, see the commit id and you do: git show <commit id> You read the commit message and there's just gibberish and nothing's explaining *why* that change was done. And you start scratching your head, trying to figure out why. Because the damn commit message is worth sh*t. This happens to us maintainers at least once a week. Well, I don't want that to happen in my tree anymore. You catch my drift? :)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c index 23d25724bae4..08bf20c60111 100644 --- a/drivers/edac/i7core_edac.c +++ b/drivers/edac/i7core_edac.c @@ -376,7 +376,7 @@ static const struct pci_id_table pci_dev_table[] = { PCI_ID_TABLE_ENTRY(pci_dev_descr_i7core_nehalem), PCI_ID_TABLE_ENTRY(pci_dev_descr_lynnfield), PCI_ID_TABLE_ENTRY(pci_dev_descr_i7core_westmere), - {0,} /* 0 terminated list. */ + {NULL,} /* 0 terminated list. */ }; /* diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c index 0c779a0326b6..a3f50a66de33 100644 --- a/drivers/edac/sb_edac.c +++ b/drivers/edac/sb_edac.c @@ -439,7 +439,7 @@ static const struct pci_id_descr pci_dev_descr_sbridge[] = { static const struct pci_id_table pci_dev_descr_sbridge_table[] = { PCI_ID_TABLE_ENTRY(pci_dev_descr_sbridge, ARRAY_SIZE(pci_dev_descr_sbridge), 1, SANDY_BRIDGE), - {0,} /* 0 terminated list. */ + {NULL,} /* 0 terminated list. */ }; /* This changes depending if 1HA or 2HA: @@ -505,7 +505,7 @@ static const struct pci_id_descr pci_dev_descr_ibridge[] = { static const struct pci_id_table pci_dev_descr_ibridge_table[] = { PCI_ID_TABLE_ENTRY(pci_dev_descr_ibridge, 12, 2, IVY_BRIDGE), - {0,} /* 0 terminated list. */ + {NULL,} /* 0 terminated list. */ }; /* Haswell support */ @@ -576,7 +576,7 @@ static const struct pci_id_descr pci_dev_descr_haswell[] = { static const struct pci_id_table pci_dev_descr_haswell_table[] = { PCI_ID_TABLE_ENTRY(pci_dev_descr_haswell, 13, 2, HASWELL), - {0,} /* 0 terminated list. */ + {NULL,} /* 0 terminated list. */ }; /* Knight's Landing Support */ @@ -620,7 +620,7 @@ static const struct pci_id_descr pci_dev_descr_knl[] = { static const struct pci_id_table pci_dev_descr_knl_table[] = { PCI_ID_TABLE_ENTRY(pci_dev_descr_knl, ARRAY_SIZE(pci_dev_descr_knl), 1, KNIGHTS_LANDING), - {0,} + {NULL,} }; /* @@ -686,7 +686,7 @@ static const struct pci_id_descr pci_dev_descr_broadwell[] = { static const struct pci_id_table pci_dev_descr_broadwell_table[] = { PCI_ID_TABLE_ENTRY(pci_dev_descr_broadwell, 10, 2, BROADWELL), - {0,} /* 0 terminated list. */ + {NULL,} /* 0 terminated list. */ };
Sparse static analysis tools generate a warning with this message "Using plain integer as NULL pointer". In this case this warning is being shown because we are trying to initialize pointer to NULL using integer value 0. Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com> --- drivers/edac/i7core_edac.c | 2 +- drivers/edac/sb_edac.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-)