Skip to content

gcode: M30 - Properly handle absolute paths and relative paths#5190

Open
bkerler wants to merge 1 commit intoprusa3d:masterfrom
bkerler:m30_cmd
Open

gcode: M30 - Properly handle absolute paths and relative paths#5190
bkerler wants to merge 1 commit intoprusa3d:masterfrom
bkerler:m30_cmd

Conversation

@bkerler
Copy link
Copy Markdown

@bkerler bkerler commented Mar 23, 2026

This fixes issue #5005.

The M30 command (delete file on SD/USB) was failing because it didn't properly handle:

  • Leading whitespace in the filename argument
  • Simplify3D format (file size appended after space)
  • Absolute paths that already include /usb/ or /sd/ prefix

*/
void GcodeSuite::M30() {
// Skip leading whitespace
const char *filename = parser.string_arg;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you need to test for NULL first?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right.


// Handle Simplify3D format which includes file size after a space (same as M23)
for (char *fn = const_cast<char *>(filename); *fn; ++fn) {
if (*fn == ' ') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does the firmware handle escape characters? That is, if I type in hello\ world.gcode, will it recognize it the entire filename? Does the firmware allow the user to upload files with spaces in their names?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The gcode parser has no escape character mechanism. The truncation-at-space is intentional — it mirrors what M23 does for Simplify3D compatibility (where the host appends a file size, e.g. M30 filename.gcode 12345).

}

// Skip empty filename
if (!filename || !*filename) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The !filename test is too late here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

if we need to check for nullptr, then yes


// Handle paths that may already have /usb/ or /sd/ prefix
if (strncmp(filename, "/usb/", 5) == 0 || strncmp(filename, "/sd/", 4) == 0) {
filepath.append_string(filename);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You need to test whether the filename is too long to fit into filepath.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ArrayStringBuilder already tracks overflows, but yeah, will check the is_ok function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants