boot_serial: minor fixes in 'erase_range()' log output and for 'bs_list()' #1803
Conversation
pepe2k
force-pushed
the
main__boot_serial-fixes
branch
from
86d490a
to
04adb5c
Compare
The 'buf' and 'len' parameters aren't used inside 'bs_list()' function. Signed-off-by: Piotr Dymacz <pepe2k@gmail.com>
pepe2k
force-pushed
the
main__boot_serial-fixes
branch
from
04adb5c
to
a09b190
Compare
pepe2k
force-pushed
the
main__boot_serial-fixes
branch
from
a09b190
to
3f26aae
Compare
I've added that in second commit ( Thanks! |
|
Format needs updating for CI to pass |
| @@ -596,8 +596,13 @@ static off_t erase_range(const struct flash_area *fap, off_t start, off_t end) | |||
| } | |||
|
|
|||
| size = flash_sector_get_off(§) + flash_sector_get_size(§) - start; | |||
| BOOT_LOG_INF("Erasing range 0x%jx:0x%jx", (intmax_t)start, | |||
| (intmax_t)(start + size - 1)); | |||
| #if defined(__ZEPHYR__) && defined(CONFIG_MINIMAL_LIBC) && defined(CONFIG_CBPRINTF_NANO) | |||
There was a problem hiding this comment.
No. Avoid Zephyr specific CONFIG_ things in code.
You can define two different formatting strings on top of the unit and guard them with CONIFG.
Probably using inttypes.h definitions for PRI would be helpful here.
Maybe we need to change the string to avoid using intmax at all if possible, at all the pointer size should be known prior to compilation.
There was a problem hiding this comment.
No. Avoid Zephyr specific
CONFIG_things in code.
OK. Let me think about alternative solution.
| @@ -240,7 +240,7 @@ bs_list_img_ver(char *dst, int maxlen, struct image_version *ver) | |||
| * List images. | |||
| */ | |||
| static void | |||
| bs_list(char *buf, int len) | |||
| bs_list(void) | |||
There was a problem hiding this comment.
There are conditionally compiled in calls, like bs_set, that still take both parameters.
There was a problem hiding this comment.
@de-nordic yes, but the bs_set() makes use of these parameters, the bs_list() doesn't. Am I not seeing something obvious here?
There was a problem hiding this comment.
Ah, sorry. I have looked at diffs quickly and missed that the bs_set is called from different function.
Currently, log info in 'erase_range()' and 'bs_upload()' shows range being erased/written relative to selected flash area which might be misleading. Include flash area offset in output so that the absolute addresses of the range being erased/written are shown. Signed-off-by: Piotr Dymacz <pepe2k@gmail.com>
The Zephyr's 'Minimal C library' doesn't support the '%j' length modifier when 'cbprintf' is configured with minimum features. Signed-off-by: Piotr Dymacz <pepe2k@gmail.com>
pepe2k
force-pushed
the
main__boot_serial-fixes
branch
from
3f26aae
to
5045928
Compare
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
This short series includes 3 minor fixes for
boot_serial:drop redundant parameters from
bs_list()The
bufandlenparameters aren't used insidebs_list()function.boot_serial: include flash area offset in log info
Currently, log info in
erase_range()andbs_upload()shows range being erased/written relative to selected flash area which might be misleading. Include flash area offset in output so that the absolute addresses of the range being erased/written are shown.don't use
%jlength modifier with Zephyr's minimal libcThe Zephyr's 'Minimal C library' doesn't support the
%jlength modifier whencbprintfis configured with minimum features.