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

Add error message if --force fails #57

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add error message if --force fails #57

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
515717a
added additional info and error message for --force option
playduck Aug 31, 2022
88ae308
added missing dash in USB-CDC string
playduck Aug 31, 2022
0510533
added another missing dash in USB-CDC string
playduck Aug 31, 2022
6acb4f8
adjusted snprintf buffer length to appropriate length
playduck Sep 1, 2022
4800ccc
removed redundant "found" in string
playduck Sep 1, 2022
e4374e7
Merge remote-tracking branch 'upstream/master'
playduck Feb 19, 2023
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: 12 additions & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,13 @@ TARGET SELECTION:
--address <addr>
Filter devices by USB device address
-f, --force
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing
the command (unless the command itself is a 'reboot') the device will be rebooted back to application mode
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the
command (unless the command itself is a 'reboot') the device will be rebooted back to application mode. Make sure the device
is using USB-CDC (USB stdio)
-F, --force-no-reboot
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing
the command (unless the command itself is a 'reboot') the device will be left connected and accessible to picotool, but
without the RPI-RP2 drive mounted
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the
command (unless the command itself is a 'reboot') the device will be left connected and accessible to picotool, but without
the RPI-RP2 drive mounted. Make sure the device is using USB-CDC (USB stdio)
To target a file
<filename>
The file name
Expand Down Expand Up @@ -213,12 +214,13 @@ OPTIONS:
--address <addr>
Filter devices by USB device address
-f, --force
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing
the command (unless the command itself is a 'reboot') the device will be rebooted back to application mode
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the
command (unless the command itself is a 'reboot') the device will be rebooted back to application mode. Make sure the device
is using USB CDC (USB stdio)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing hyphen in USB-CDC here?

-F, --force-no-reboot
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing
the command (unless the command itself is a 'reboot') the device will be left connected and accessible to picotool, but
without the RPI-RP2 drive mounted
Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the
command (unless the command itself is a 'reboot') the device will be left connected and accessible to picotool, but without
the RPI-RP2 drive mounted. Make sure the device is using USB CDC (USB stdio)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing hyphen in USB-CDC here?

Copy link
Contributor

Choose a reason for hiding this comment

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

No hyphen is needed (example) and I don't think I've ever seen it written with a hyphen. I usually write it as "USB CDC ACM".

File to save to
<filename>
The file name
Expand Down
25 changes: 15 additions & 10 deletions main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ auto device_selection =
(option("--address") & integer("addr").min_value(1).max_value(127).set(settings.address)
.if_missing([] { return "missing address"; })) % "Filter devices by USB device address"
#if !defined(_WIN32)
+ option('f', "--force").set(settings.force) % "Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the command (unless the command itself is a 'reboot') the device will be rebooted back to application mode" +
option('F', "--force-no-reboot").set(settings.force_no_reboot) % "Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the command (unless the command itself is a 'reboot') the device will be left connected and accessible to picotool, but without the RPI-RP2 drive mounted"
+ option('f', "--force").set(settings.force) % "Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the command (unless the command itself is a 'reboot') the device will be rebooted back to application mode. Make sure the device is using USB-CDC (USB stdio)" +
option('F', "--force-no-reboot").set(settings.force_no_reboot) % "Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the command (unless the command itself is a 'reboot') the device will be left connected and accessible to picotool, but without the RPI-RP2 drive mounted. Make sure the device is using USB-CDC (USB stdio)"
#endif
).min(0).doc_non_optional(true);

Expand Down Expand Up @@ -1564,26 +1564,31 @@ void info_guts(memory_access &raw_access) {
}

string missing_device_string(bool wasRetry) {
char b[256];
const size_t bufferLen = 512;
char b[bufferLen];
if (wasRetry) {
strcpy(b, "Despite the reboot attempt, no ");
strncpy(b, "Despite the reboot attempt, no ", bufferLen);
} else {
strcpy(b, "No ");
strncpy(b, "No ", bufferLen);
}
char *buf = b + strlen(b);
char *bufErrorMessage = b + strnlen(b, bufferLen);
if (settings.address != -1) {
if (settings.bus != -1) {
sprintf(buf, "accessible RP2040 device in BOOTSEL mode was found at bus %d, address %d.", settings.bus, settings.address);
snprintf(bufErrorMessage, bufferLen, "accessible RP2040 device in BOOTSEL mode was found at bus %d, address %d.", settings.bus, settings.address);
} else {
sprintf(buf, "accessible RP2040 devices in BOOTSEL mode were found with address %d.", settings.address);
snprintf(bufErrorMessage, bufferLen, "accessible RP2040 devices in BOOTSEL mode were found with address %d.", settings.address);
}
} else {
if (settings.bus != -1) {
sprintf(buf, "accessible RP2040 devices in BOOTSEL mode were found found on bus %d.", settings.bus);
snprintf(bufErrorMessage, bufferLen, "accessible RP2040 devices in BOOTSEL mode were found found on bus %d.", settings.bus);
} else {
sprintf(buf, "accessible RP2040 devices in BOOTSEL mode were found.");
snprintf(bufErrorMessage, bufferLen, "accessible RP2040 devices in BOOTSEL mode were found.");
}
}
if (settings.force) {
char* bufForceError = b + strnlen(b, bufferLen);
snprintf(bufForceError, bufferLen, "\nTo force a device into BOOTSEL mode, make sure the RP2040 is configured to use USB-CDC (USB stdio).");
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're using snprintf to prevent string-overflows, do you also need to pass in a smaller value than bufferLen if the string b already contains some content?

Copy link
Author

Choose a reason for hiding this comment

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

True, the same holds for the bufErrorMessage strings above.

The length argument should be something along the lines of bufferLen - strnlen(b, bufferLen). I'm not sure if this causes an off-by-one error regarding the null terminator (e.g. should it be bufferLen - strnlen(b, bufferLen) - 1 or not)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this causes an off-by-one error

I dunno either 🤷‍♂️ I'm more of a Python programmer than a C programmer 😉

}
return b;
}

Expand Down