On 7 Feb 2020, at 10:22, Scott Long <sco...@freebsd.org> wrote: > > Author: scottl > Date: Fri Feb 7 09:22:08 2020 > New Revision: 357647 > URL: https://svnweb.freebsd.org/changeset/base/357647 > > Log: > Ever since the block layer expanded its command syntax beyond just > BIO_READ and BIO_WRITE, we've handled this expanded syntax poorly in > drivers when the driver doesn't support a particular command. Do a > sweep and fix that. ... > Modified: head/sys/dev/altera/sdcard/altera_sdcard_io.c > ============================================================================== > --- head/sys/dev/altera/sdcard/altera_sdcard_io.c Fri Feb 7 08:39:00 > 2020 (r357646) > +++ head/sys/dev/altera/sdcard/altera_sdcard_io.c Fri Feb 7 09:22:08 > 2020 (r357647) > @@ -293,27 +293,27 @@ recheck: > } > > static void > -altera_sdcard_io_start_internal(struct altera_sdcard_softc *sc, struct bio > *bp) > +altera_sdcard_io_start_internal(struct altera_sdcard_softc *sc, struct bio > **bp) > { > > - switch (bp->bio_cmd) { > + switch (*bp->bio_cmd) { > case BIO_READ: > - altera_sdcard_write_cmd_arg(sc, bp->bio_pblkno * > + altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno * > ALTERA_SDCARD_SECTORSIZE); > altera_sdcard_write_cmd(sc, ALTERA_SDCARD_CMD_READ_BLOCK); > break; > > case BIO_WRITE: > - altera_sdcard_write_rxtx_buffer(sc, bp->bio_data, > - bp->bio_bcount); > - altera_sdcard_write_cmd_arg(sc, bp->bio_pblkno * > + altera_sdcard_write_rxtx_buffer(sc, *bp->bio_data, > + *bp->bio_bcount); > + altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno * > ALTERA_SDCARD_SECTORSIZE); > altera_sdcard_write_cmd(sc, ALTERA_SDCARD_CMD_WRITE_BLOCK); > break; > > default: > - panic("%s: unsupported I/O operation %d", __func__, > - bp->bio_cmd); > + biofinish(*bp, NULL, EOPNOTSUPP); > + *bp = NULL; > } > } >
This gets the indirections wrong, leading to errors with at least clang 10:
sys/dev/altera/sdcard/altera_sdcard_io.c: In function
'altera_sdcard_io_start_internal':
sys/dev/altera/sdcard/altera_sdcard_io.c:299:13: error: '*bp' is a pointer; did
you mean to use '->'?
switch (*bp->bio_cmd) {
^~
->
sys/dev/altera/sdcard/altera_sdcard_io.c:301:38: error: '*bp' is a pointer; did
you mean to use '->'?
altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno *
^~
->
sys/dev/altera/sdcard/altera_sdcard_io.c:307:42: error: '*bp' is a pointer; did
you mean to use '->'?
altera_sdcard_write_rxtx_buffer(sc, *bp->bio_data,
^~
->
sys/dev/altera/sdcard/altera_sdcard_io.c:308:10: error: '*bp' is a pointer; did
you mean to use '->'?
*bp->bio_bcount);
^~
->
sys/dev/altera/sdcard/altera_sdcard_io.c:309:38: error: '*bp' is a pointer; did
you mean to use '->'?
altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno *
^~
->
In this case, (*bp)->fieldname is the correct way to specify the struct fields.
> @@ -332,8 +332,8 @@ altera_sdcard_io_start(struct altera_sdcard_softc *sc,
> */
> KASSERT(bp->bio_bcount == ALTERA_SDCARD_SECTORSIZE,
> ("%s: I/O size not %d", __func__, ALTERA_SDCARD_SECTORSIZE));
> - altera_sdcard_io_start_internal(sc, bp);
> - sc->as_currentbio = bp;
> + altera_sdcard_io_start_internal(sc, &bp);
> + sc->as_currentbio = *bp;
> sc->as_retriesleft = ALTERA_SDCARD_RETRY_LIMIT;
> }
And a similar error here:
sys/dev/altera/sdcard/altera_sdcard_io.c: In function 'altera_sdcard_io_start':
sys/dev/altera/sdcard/altera_sdcard_io.c:336:20: error: incompatible types when
assigning to type 'struct bio *' from type 'struct bio'
sc->as_currentbio = *bp;
^
As sc->as_currentbio is already a pointer, there is no need to dereference bp
again.
I submitted https://reviews.freebsd.org/D23730 to get this fixed.
-Dimitry
signature.asc
Description: Message signed with OpenPGP
