Skip to content

Add image string information to the display output of the fw-info subcommand #339

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

Open
hsinliang wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add image string information to the display output of the fw-info subcommand #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion cli/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1770,17 +1770,37 @@ static const char *fw_active_string(struct switchtec_fw_image_info *inf)
return inf->active ? " - Active" : "";
}

static void char_arr_print(char *buf, size_t len)
{
int i;

for (i = 0; i < len; i++)
Copy link
Collaborator

@lsgunth lsgunth Jul 22, 2025

Choose a reason for hiding this comment

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

Please follow the coding style: should use tab characters (set to 8 spaces) as indents and the { for for and if statements should be on the same line.

This project follows Linux coding style so see here for more information:
https://www.kernel.org/doc/html/latest/process/coding-style.html

{
printf("%c", buf[i]);
if (buf[i] == 0)
{
break;
}
}
}

static void print_fw_part_line(const char *tag,
struct switchtec_fw_image_info *inf)
{
if (!inf)
return;

printf(" %-4s\tVersion: %-8s\tCRC: %08lx\t%4s%11s%s\n",
printf(" %-4s\tVersion: %-8s\tCRC: %08lx\t%4s%11s%s ",
tag, inf->version, inf->image_crc,
inf->read_only ? "(RO)" : "",
inf->running ? " (Running)" : "",
inf->valid ? "" : " (Invalid)");

if(inf->valid){
printf("Image String:");
Copy link
Collaborator

@lsgunth lsgunth Jul 22, 2025

Choose a reason for hiding this comment

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

The lines here are going to be too long for a standard screen. Can we maybe add this to a new line and match the indent of the Version string?

if (inf->valid) {
    printf("  %-4s\tImage String: %.*s\n", "", sizeof(inf->img_str), 
              inf->img_str);

char_arr_print(inf->img_str, sizeof(inf->img_str));
Copy link
Collaborator

@lsgunth lsgunth Jul 22, 2025

Choose a reason for hiding this comment

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

This doesn't actually need the new helper, it should be equivalent to:

printf("%.*s", sizeof(inf->img_str), inf->img_str);

}
printf("\n");
}

static int print_fw_part_info(struct switchtec_dev *dev)
Expand Down
1 change: 1 addition & 0 deletions inc/switchtec/switchtec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ struct switchtec_fw_image_info {
size_t part_body_offset; //!< Partition image body offset
size_t image_len; //!< Length of the image
unsigned long image_crc; //!< CRC checksum of the image
char img_str[16];
Copy link
Collaborator

@lsgunth lsgunth Jul 22, 2025

Choose a reason for hiding this comment

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

NIT: I'd very much prefer changes to the library be put in separate patches from changes to the CLI.


bool valid;
bool active;
Expand Down
4 changes: 4 additions & 0 deletions lib/fw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,7 @@ static int switchtec_fw_info_metadata_gen5(struct switchtec_dev *dev,
.part_id = inf->part_id,
};
int ret;
int i;

if (inf->part_id == SWITCHTEC_FW_PART_ID_G5_NVLOG)
return 1;
Expand Down Expand Up @@ -1095,6 +1096,9 @@ static int switchtec_fw_info_metadata_gen5(struct switchtec_dev *dev,
inf->image_len = le32toh(metadata->image_len);
inf->metadata = metadata;

for(i = 0; i< 16; i++)
inf->img_str[i] = metadata->img_str[i];
Copy link
Collaborator

@lsgunth lsgunth Jul 22, 2025

Choose a reason for hiding this comment

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

This can be a memcpy() and shouldn't need the constant 16:

memcpy(inf->img_str, metadata->img_str, sizeof(inf->img_str));


return 0;

err_out:
Expand Down