Re: [v12, 7/8] base: soc: introduce soc_device_match() interface
On 2016-09-21 09:56, Alexander Shiyan wrote: >> Среда, 21 сентября 2016, 9:57 +03:00 от Yangbo Lu: >> >> From: Arnd Bergmann < a...@arndb.de > >> >> We keep running into cases where device drivers want to know the exact >> version of the a SoC they are currently running on. In the past, this has >> usually been done through a vendor specific API that can be called by a >> driver, or by directly accessing some kind of version register that is >> not part of the device itself but that belongs to a global register area >> of the chip. > ... >> +const struct soc_device_attribute *soc_device_match( >> +const struct soc_device_attribute *matches) >> +{ >> +int ret = 0; >> + >> +if (!matches) >> +return NULL; >> + >> +while (!ret) { >> +if (!(matches->machine || matches->family || >> + matches->revision || matches->soc_id)) >> +break; >> +ret = bus_for_each_dev(_bus_type, NULL, (void *)matches, >> + soc_device_match_one); >> +if (!ret) >> +matches++; > > So, what happen if next "matches" (after increment) will be NULL? A crash? > I think you should use while(matches) at the start of this procedure. *arrgh* *If* matches wrap, you indeed have *big* problems. *Elsewhere* Hint: Please read the review comments on the previous version of this series [1] before commenting further. Cheers, Peter [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg126617.html
Re: [v12, 7/8] base: soc: introduce soc_device_match() interface
On 2016-09-21 09:56, Alexander Shiyan wrote: >> Среда, 21 сентября 2016, 9:57 +03:00 от Yangbo Lu : >> >> From: Arnd Bergmann < a...@arndb.de > >> >> We keep running into cases where device drivers want to know the exact >> version of the a SoC they are currently running on. In the past, this has >> usually been done through a vendor specific API that can be called by a >> driver, or by directly accessing some kind of version register that is >> not part of the device itself but that belongs to a global register area >> of the chip. > ... >> +const struct soc_device_attribute *soc_device_match( >> +const struct soc_device_attribute *matches) >> +{ >> +int ret = 0; >> + >> +if (!matches) >> +return NULL; >> + >> +while (!ret) { >> +if (!(matches->machine || matches->family || >> + matches->revision || matches->soc_id)) >> +break; >> +ret = bus_for_each_dev(_bus_type, NULL, (void *)matches, >> + soc_device_match_one); >> +if (!ret) >> +matches++; > > So, what happen if next "matches" (after increment) will be NULL? A crash? > I think you should use while(matches) at the start of this procedure. *arrgh* *If* matches wrap, you indeed have *big* problems. *Elsewhere* Hint: Please read the review comments on the previous version of this series [1] before commenting further. Cheers, Peter [1] https://www.mail-archive.com/netdev@vger.kernel.org/msg126617.html
Re: [v12, 7/8] base: soc: introduce soc_device_match() interface
>Среда, 21 сентября 2016, 9:57 +03:00 от Yangbo Lu: > >From: Arnd Bergmann < a...@arndb.de > > >We keep running into cases where device drivers want to know the exact >version of the a SoC they are currently running on. In the past, this has >usually been done through a vendor specific API that can be called by a >driver, or by directly accessing some kind of version register that is >not part of the device itself but that belongs to a global register area >of the chip. ... >+const struct soc_device_attribute *soc_device_match( >+const struct soc_device_attribute *matches) >+{ >+int ret = 0; >+ >+if (!matches) >+return NULL; >+ >+while (!ret) { >+if (!(matches->machine || matches->family || >+ matches->revision || matches->soc_id)) >+break; >+ret = bus_for_each_dev(_bus_type, NULL, (void *)matches, >+ soc_device_match_one); >+if (!ret) >+matches++; So, what happen if next "matches" (after increment) will be NULL? I think you should use while(matches) at the start of this procedure. ---
Re: [v12, 7/8] base: soc: introduce soc_device_match() interface
>Среда, 21 сентября 2016, 9:57 +03:00 от Yangbo Lu : > >From: Arnd Bergmann < a...@arndb.de > > >We keep running into cases where device drivers want to know the exact >version of the a SoC they are currently running on. In the past, this has >usually been done through a vendor specific API that can be called by a >driver, or by directly accessing some kind of version register that is >not part of the device itself but that belongs to a global register area >of the chip. ... >+const struct soc_device_attribute *soc_device_match( >+const struct soc_device_attribute *matches) >+{ >+int ret = 0; >+ >+if (!matches) >+return NULL; >+ >+while (!ret) { >+if (!(matches->machine || matches->family || >+ matches->revision || matches->soc_id)) >+break; >+ret = bus_for_each_dev(_bus_type, NULL, (void *)matches, >+ soc_device_match_one); >+if (!ret) >+matches++; So, what happen if next "matches" (after increment) will be NULL? I think you should use while(matches) at the start of this procedure. ---