On Mon, 26 Nov 2018 16:42:48 +0530 Jagan Teki <ja...@openedev.com> wrote:
> On 26/11/18 2:12 PM, Boris Brezillon wrote: > > Hi Jagan, > > > > On Thu, 22 Nov 2018 09:40:56 +0100 > > Boris Brezillon <boris.brezil...@bootlin.com> wrote: > > > >>>> + /* > >>>> + * Setting mtd->priv to NULL is the best we can do. Thanks to > >>>> that, > >>>> + * the MTD layer can still call mtd hooks without risking a > >>>> + * use-after-free bug. Still, things should be fixed to prevent > >>>> the > >>>> + * spi_flash object from being destroyed when del_mtd_device() > >>>> fails. > >>>> + */ > >>>> + sf_mtd_info.priv = NULL; > >>>> + printf("Failed to unregister MTD %s and the spi_flash object is > >>>> going away: you're in deep trouble!", > >>>> + sf_mtd_info.name); > >>> > >>> Why do we need this print? > >> > >> Yes we do, just to keep the user informed that something bad happened > >> and its spi-flash is no longer usable (at least through the MTD layer). > >> > >>> can't we do the same thing in MTD core > >>> itself, so-that it can be generic for all flash objects. > >> > >> del_mtd_device() can fail, so it's the caller responsibility to decide > >> what to do when that happens. Some users will propagate the error to > >> the upper layer and maybe cancel the device removal (AFAICT, > >> driver->remove() can return an error, not sure what happens in this > >> case though). For others, like spi-flash, the device will go away, and > >> all subsequent accesses will fail. > > > > I'm about to send a new version fixing the problem I mentioned in patch > > 3, but before doing that, I'd like to know if my answer convinced you or > > if you'd still prefer this message to go away (or be placed in > > mtdcore/mtdpart.c). > > > I'm thinking of having the message still in MTD by showing which > interface it would belongs, along with the details. Then we'd need something less _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot