Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

boot_serial: minor fixes in 'erase_range()' log output and for 'bs_list()' #1803

Closed
wants to merge 3 commits into from
Closed

boot_serial: minor fixes in 'erase_range()' log output and for 'bs_list()' #1803

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e974c8f
boot_serial: drop redundant parameters from 'bs_list()'
pepe2k Jun 27, 2023
5785f1f
boot_serial: include flash area offset in log info
pepe2k Sep 8, 2023
5045928
boot_serial: don't use '%j' length modifier with Zephyr's minimal libc
pepe2k Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 12 additions & 6 deletions boot/boot_serial/src/boot_serial.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are conditionally compiled in calls, like bs_set, that still take both parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@de-nordic yes, but the bs_set() makes use of these parameters, the bs_list() doesn't. Am I not seeing something obvious here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry. I have looked at diffs quickly and missed that the bs_set is called from different function.

{
struct image_header hdr;
uint32_t slot, area_id;
Expand Down Expand Up @@ -521,7 +521,7 @@ bs_set(char *buf, int len)
out:
if (rc == 0) {
/* Success - return updated list of images */
bs_list(buf, len);
bs_list();
} else {
/* Error code, only return the error */
zcbor_map_start_encode(cbor_state, 10);
Expand Down Expand Up @@ -551,7 +551,7 @@ static void
bs_list_set(uint8_t op, char *buf, int len)
{
if (op == NMGR_OP_READ) {
bs_list(buf, len);
bs_list();
} else {
#ifdef MCUBOOT_SERIAL_IMG_GRP_IMAGE_STATE
bs_set(buf, len);
Expand Down Expand Up @@ -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(&sect) + flash_sector_get_size(&sect) - 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@de-nordic

No. Avoid Zephyr specific CONFIG_ things in code.

OK. Let me think about alternative solution.

BOOT_LOG_INF("Erasing range 0x%lx:0x%lx", (fap->fa_off + start),
(fap->fa_off + start + size - 1));
#else
BOOT_LOG_INF("Erasing range 0x%jx:0x%jx", (intmax_t)(fap->fa_off + start),
(intmax_t)(fap->fa_off + start + size - 1));
#endif

rc = flash_area_erase(fap, start, size);
if (rc != 0) {
Expand Down Expand Up @@ -781,7 +786,8 @@ bs_upload(char *buf, int len)
rem_bytes = 0;
}

BOOT_LOG_INF("Writing at 0x%x until 0x%x", curr_off, curr_off + img_chunk_len);
BOOT_LOG_INF("Writing at 0x%lx until 0x%lx", fap->fa_off + curr_off,
fap->fa_off + curr_off + img_chunk_len);
/* Write flash aligned chunk, note that img_chunk_len now holds aligned length */
#if defined(MCUBOOT_SERIAL_UNALIGNED_BUFFER_SIZE) && MCUBOOT_SERIAL_UNALIGNED_BUFFER_SIZE > 0
if (flash_area_align(fap) > 1 &&
Expand Down