Re: [flashrom] [PATCH] cbtable mapping fix

[Stefan Reinauer]

* Carl-Daniel Hailfinger  [101007 03:09]:
> The cbtable code in flashrom is ancient, and although it grew new
> features over time, it is not exactly one of the most readable pieces of
> code in flashrom. This is in part due to the complicated integration of
> the cbtable code in flashrom, and in part due to the way mapping the
> cbtable has to deal with OS constraints.
> The goal for cbtable and DMI code is to be well-encapsulated with equal
> rights for both.
>
> I believe the following patch is a bugfix, but I'd love to get a very
> thorough review for this.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-cbtable_mapping_fixup/cbtable.c
>
> --- flashrom-cbtable_mapping_fixup/cbtable.c	(Revision 1199)
> +++ flashrom-cbtable_mapping_fixup/cbtable.c	(Arbeitskopie)
> @@ -203,8 +203,11 @@
>    struct lb_record *rec, *last;
>
> #ifdef __DARWIN__
> /* This is a hack. DirectHW fails to map physical address 0x00000000.
> * Why?
> /* This is a hack. DirectHW apparently fails to map physical address
> * 0x00000000.
> * FIXME: Modify DirectHW so that it returns (void *)-1 as error
> * instead of NULL. Maybe the mapping works and earlier failure reports
> * were just side effects of a direct mapping at 0x00000000.
> */

Unfortunately not. The issues are unrelated. (so the FIXME should be
dropped)
>     lb_table->table_entries);
>
> search_lb_records(rec, last, addr + lb_table->header_bytes);
> /* FIXME: unmap the table area here or will any code access it later? */

Just put the actual unmap there. The pointer to the mapped area is a
local variable so unless some other code is broken in much more evil
ways, noone will (nor should, that's the point of this code) access the
tables later.

Stefan