Hello T Michael Turney,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32287
to review the following change.
Change subject: lib/fmap: Add area read/write functions ......................................................................
lib/fmap: Add area read/write functions
Change-Id: I7669b8dc07b1aa5f00e7d8d0b1305b3de6c5949c Signed-off-by: T Michael Turney mturney@codeaurora.org --- M src/include/fmap.h M src/lib/fmap.c 2 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/32287/1
diff --git a/src/include/fmap.h b/src/include/fmap.h index 5834831..ab7e5ab 100644 --- a/src/include/fmap.h +++ b/src/include/fmap.h @@ -40,4 +40,12 @@ * Return 0 on success, < 0 on error. */ int fmap_find_region_name(const struct region * const ar, char name[FMAP_STRLEN]); + +/* Read fmap area into provided buffer. + * Return size read on success, < 0 on error. */ +ssize_t fmap_read_area(const char *name, void *buffer, size_t size); + +/* Write provided buffer into fmap area. + * Return size written on success, < 0 on error. */ +ssize_t fmap_overwrite_area(const char *name, const void *buffer, size_t size); #endif diff --git a/src/lib/fmap.c b/src/lib/fmap.c index 9602134..e18f406 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -173,3 +173,25 @@
return -1; } + +ssize_t fmap_read_area(const char *name, void *buffer, size_t size) +{ + struct region_device rdev; + if (fmap_locate_area_as_rdev(name, &rdev)) + return -1; + return rdev_readat(&rdev, buffer, 0, + MIN(size, region_device_sz(&rdev))); +} + +ssize_t fmap_overwrite_area(const char *name, const void *buffer, size_t size) +{ + struct region_device rdev; + + if (fmap_locate_area_as_rdev_rw(name, &rdev)) + return -1; + if (size > region_device_sz(&rdev)) + return -1; + if (rdev_eraseat(&rdev, 0, region_device_sz(&rdev)) == -1) + return -1; + return rdev_writeat(&rdev, (const void *)buffer, 0, size); +}
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32287 )
Change subject: lib/fmap: Add area read/write functions ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c File src/lib/fmap.c:
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c@194 PS1, Line 194: == -1) Let's check for < 0 to be safe
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c@196 PS1, Line 196: (const void *) What is this cast doing?
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32287 )
Change subject: lib/fmap: Add area read/write functions ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c File src/lib/fmap.c:
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c@194 PS1, Line 194: == -1)
Let's check for < 0 to be safe
Ack
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c@196 PS1, Line 196: (const void *)
What is this cast doing?
The cast is to satisfy the prototype in commonlib/region.h.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32287 )
Change subject: lib/fmap: Add area read/write functions ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c File src/lib/fmap.c:
https://review.coreboot.org/#/c/32287/1/src/lib/fmap.c@196 PS1, Line 196: (const void *)
The cast is to satisfy the prototype in commonlib/region.h.
But... 'buffer' already is a (const void *)? I don't get it.
Hello T Michael Turney, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32287
to look at the new patch set (#2).
Change subject: lib/fmap: Add area read/write functions ......................................................................
lib/fmap: Add area read/write functions
Change-Id: I7669b8dc07b1aa5f00e7d8d0b1305b3de6c5949c Signed-off-by: T Michael Turney mturney@codeaurora.org --- M src/include/fmap.h M src/lib/fmap.c 2 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/32287/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32287 )
Change subject: lib/fmap: Add area read/write functions ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32287 )
Change subject: lib/fmap: Add area read/write functions ......................................................................
lib/fmap: Add area read/write functions
Change-Id: I7669b8dc07b1aa5f00e7d8d0b1305b3de6c5949c Signed-off-by: T Michael Turney mturney@codeaurora.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32287 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/include/fmap.h M src/lib/fmap.c 2 files changed, 30 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/include/fmap.h b/src/include/fmap.h index 5834831..ab7e5ab 100644 --- a/src/include/fmap.h +++ b/src/include/fmap.h @@ -40,4 +40,12 @@ * Return 0 on success, < 0 on error. */ int fmap_find_region_name(const struct region * const ar, char name[FMAP_STRLEN]); + +/* Read fmap area into provided buffer. + * Return size read on success, < 0 on error. */ +ssize_t fmap_read_area(const char *name, void *buffer, size_t size); + +/* Write provided buffer into fmap area. + * Return size written on success, < 0 on error. */ +ssize_t fmap_overwrite_area(const char *name, const void *buffer, size_t size); #endif diff --git a/src/lib/fmap.c b/src/lib/fmap.c index 9602134..2b4e3ba 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -173,3 +173,25 @@
return -1; } + +ssize_t fmap_read_area(const char *name, void *buffer, size_t size) +{ + struct region_device rdev; + if (fmap_locate_area_as_rdev(name, &rdev)) + return -1; + return rdev_readat(&rdev, buffer, 0, + MIN(size, region_device_sz(&rdev))); +} + +ssize_t fmap_overwrite_area(const char *name, const void *buffer, size_t size) +{ + struct region_device rdev; + + if (fmap_locate_area_as_rdev_rw(name, &rdev)) + return -1; + if (size > region_device_sz(&rdev)) + return -1; + if (rdev_eraseat(&rdev, 0, region_device_sz(&rdev)) < 0) + return -1; + return rdev_writeat(&rdev, buffer, 0, size); +}
-
Julius Werner (Code Review) -
mturney mturney (Code Review)
-
Patrick Georgi (Code Review)
-
T.Michael Turney (Code Review)