Re: NULL checks of static arrays
On 08/03/16(Tue) 14:05, Michael McConville wrote:
> Martin Pieuchot wrote:
> > On 06/03/16(Sun) 19:20, Michael McConville wrote:
> > > We check static arrays against NULL pretty often in the kernel. I
> > > suspect most of these are due to recent kernel API changes. Should they
> > > be removed, or do people want to keep them around in case the APIs
> > > change again? Clang 3.7 warns about them by default, so they're easy to
> > > find.
> >
> > I think you should post diffs so we can decide on a case-by-case basis.
> >
> > But if they are leftovers from cleanups, then we should clean them.
>
> Here are a few examples I have sitting in my tree:
>
>
> Index: sys/dev/ic/ti.c
> ===
> RCS file: /cvs/src/sys/dev/ic/ti.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 ti.c
> --- sys/dev/ic/ti.c 25 Nov 2015 03:09:58 - 1.22
> +++ sys/dev/ic/ti.c 8 Mar 2016 22:02:50 -
> @@ -484,9 +484,6 @@ ti_handle_events(struct ti_softc *sc)
> struct ti_event_desc*e;
> struct ifnet*ifp = >arpcom.ac_if;
>
> - if (sc->ti_rdata->ti_event_ring == NULL)
> - return;
> -
> while (sc->ti_ev_saved_considx != sc->ti_ev_prodidx.ti_idx) {
> e = >ti_rdata->ti_event_ring[sc->ti_ev_saved_considx];
> switch (TI_EVENT_EVENT(e)) {
> @@ -846,9 +843,6 @@ ti_free_tx_ring(struct ti_softc *sc)
> {
> int i;
> struct ti_txmap_entry *entry;
> -
> - if (sc->ti_rdata->ti_tx_ring == NULL)
> - return;
>
> for (i = 0; i < TI_TX_RING_CNT; i++) {
> if (sc->ti_cdata.ti_tx_chain[i] != NULL) {
This one is ok mpi@
> Index: sys/dev/usb/uoakrh.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uoakrh.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 uoakrh.c
> --- sys/dev/usb/uoakrh.c 9 Jan 2016 04:14:42 - 1.13
> +++ sys/dev/usb/uoakrh.c 8 Mar 2016 22:02:50 -
> @@ -215,11 +215,6 @@ uoakrh_detach(struct device *self, int f
> wakeup(>sc_sensortask);
> sensordev_deinstall(>sc_sensordev);
>
> - if (>sc_sensordev != NULL) {
> - sensor_detach(>sc_sensordev, >sc_sensor.temp);
> - sensor_detach(>sc_sensordev, >sc_sensor.humi);
> - }
> -
> if (sc->sc_sensortask != NULL)
> sensor_task_unregister(sc->sc_sensortask);
>
>
You've got it backward ;)
Re: NULL checks of static arrays
Martin Pieuchot wrote:
> On 06/03/16(Sun) 19:20, Michael McConville wrote:
> > We check static arrays against NULL pretty often in the kernel. I
> > suspect most of these are due to recent kernel API changes. Should they
> > be removed, or do people want to keep them around in case the APIs
> > change again? Clang 3.7 warns about them by default, so they're easy to
> > find.
>
> I think you should post diffs so we can decide on a case-by-case basis.
>
> But if they are leftovers from cleanups, then we should clean them.
Here are a few examples I have sitting in my tree:
Index: sys/dev/ic/ti.c
===
RCS file: /cvs/src/sys/dev/ic/ti.c,v
retrieving revision 1.22
diff -u -p -r1.22 ti.c
--- sys/dev/ic/ti.c 25 Nov 2015 03:09:58 - 1.22
+++ sys/dev/ic/ti.c 8 Mar 2016 22:02:50 -
@@ -484,9 +484,6 @@ ti_handle_events(struct ti_softc *sc)
struct ti_event_desc*e;
struct ifnet*ifp = >arpcom.ac_if;
- if (sc->ti_rdata->ti_event_ring == NULL)
- return;
-
while (sc->ti_ev_saved_considx != sc->ti_ev_prodidx.ti_idx) {
e = >ti_rdata->ti_event_ring[sc->ti_ev_saved_considx];
switch (TI_EVENT_EVENT(e)) {
@@ -846,9 +843,6 @@ ti_free_tx_ring(struct ti_softc *sc)
{
int i;
struct ti_txmap_entry *entry;
-
- if (sc->ti_rdata->ti_tx_ring == NULL)
- return;
for (i = 0; i < TI_TX_RING_CNT; i++) {
if (sc->ti_cdata.ti_tx_chain[i] != NULL) {
Index: sys/dev/usb/uoakrh.c
===
RCS file: /cvs/src/sys/dev/usb/uoakrh.c,v
retrieving revision 1.13
diff -u -p -r1.13 uoakrh.c
--- sys/dev/usb/uoakrh.c9 Jan 2016 04:14:42 - 1.13
+++ sys/dev/usb/uoakrh.c8 Mar 2016 22:02:50 -
@@ -215,11 +215,6 @@ uoakrh_detach(struct device *self, int f
wakeup(>sc_sensortask);
sensordev_deinstall(>sc_sensordev);
- if (>sc_sensordev != NULL) {
- sensor_detach(>sc_sensordev, >sc_sensor.temp);
- sensor_detach(>sc_sensordev, >sc_sensor.humi);
- }
-
if (sc->sc_sensortask != NULL)
sensor_task_unregister(sc->sc_sensortask);
Re: NULL checks of static arrays
On 06/03/16(Sun) 19:20, Michael McConville wrote: > We check static arrays against NULL pretty often in the kernel. I > suspect most of these are due to recent kernel API changes. Should they > be removed, or do people want to keep them around in case the APIs > change again? Clang 3.7 warns about them by default, so they're easy to > find. I think you should post diffs so we can decide on a case-by-case basis. But if they are leftovers from cleanups, then we should clean them.
NULL checks of static arrays
We check static arrays against NULL pretty often in the kernel. I suspect most of these are due to recent kernel API changes. Should they be removed, or do people want to keep them around in case the APIs change again? Clang 3.7 warns about them by default, so they're easy to find.
