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.

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

Extension hide refactor; restore "strncpy" function alternatives #2817

Conversation

kisslorand
Copy link
Contributor

@kisslorand kisslorand commented Jul 22, 2023

Requirements

BTT or MKS TFT.

Description

  • The "strxcpy", "strscpy" and "strwcpy" alternatives for "strncpy" were restored. PR Bug fixes on top of last merged PRs #2768 eliminated them. PR Bug fixes on top of last merged PRs #2768 claims that "strxcpy is not applicable at all in VFS API". Well... "strxcpy" is EXACTLY there where it is best suited. It just adds that extra '\0' character needed for file extension hide and restoration functions.
    The author of that PR probably missed that the source string also contains the '\0' terminal character and the "strxcpy" copies only "num - 1" characters. So if the source is a filename (not a folder name) "num" equals to the number of bytes in the filename (excepting the '\0' terminal character) plus two more bytes (num = strlen(filename) + 2). If you substract 1 for "num" (becasue "num - 1" bytes will copy "strxcpy" from source to destination) you will end up to copy "num - 1" bytes from source which equals to "strlen(filename) + 1" which equals to the bytes number in the filename plus the '\0' terminator character. No risk of any overflow here.
    So not only that "strxcpy" is PERFECTLY SAFE and suitable for the job in VFS API but it is also faster compared to the offered alternative by PR Bug fixes on top of last merged PRs #2768. This also adds to the snappiness of the filelist.
    The alternatives offered by PR Bug fixes on top of last merged PRs #2768 are not as light as the ones they replaced and in some cases they fail to deliver the promised <"dest" always ends with '\0'> which results in an undefined/unsafe state of "dest" if it is not initialized beforehead. It's not the case with the functions of this PR.
  • "setPrintTitle()" is back reworked and with a very small footprint (40 bytes smaller than the original "getPrintTitle()").
  • The hide extension of the filenames got refactored resulting in a speed boost in file list procedure, by a rough estimation of at least 25% (the value is estimated, do not ask for scientific proof). The list creation and the navigation between pages in the list is more snappier. It is done by eliminating unnecessary extension hiding, restoration and nested functions.
  • The following expression type &string[strlen(string)] has been replaced by strchr(string, '\0'). The expression &string[strlen(string)] first finds the '\0' terminal character in the "string" variable counting in the meantime the bytes checked than iterates variable "string" by the bytes resulted in the previous check than it gives back the address of the iteration result, actually giving the address of the null terminator character of the "string" variable. strchr(string, '\0') does EXACTLY that with only one search, it directly gives the address of the null terminator character of the "string" variable.
  • PR Bug fixes on top of last merged PRs #2768 claimed to fix "wrong check on BedLeveling menu" but there was nothing to fix. Maybe it was just a lack of code understanding... If X can be only A or B and it is not B than what is X? :)
    Before checking if the firmware of the motherboard if it is Marlin or not there's a conditional if (levelStateNew != UNDEFINED) which limits the firmware to be either Marlin or Reprap. So if it's not Reprap, it's... you guessed right, it's Marlin! The fix of PR Bug fixes on top of last merged PRs #2768 was not a fix so it's reverted.

@kisslorand kisslorand force-pushed the Extension-hide,-str_cpy-restore branch 2 times, most recently from dd7fa9c to 9f3a3bb Compare October 9, 2023 08:36
@kisslorand kisslorand force-pushed the Extension-hide,-str_cpy-restore branch from 9f3a3bb to 828675d Compare October 9, 2023 08:36
Copy link

stale bot commented Dec 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added Abandoned and removed Abandoned labels Dec 15, 2023
Copy link

This PR has been automatically marked as stale because it has had no activity for the last 60 days. It will be closed in 7 days if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the Stale label Apr 21, 2024
@github-actions github-actions bot closed this Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant