-
Notifications
You must be signed in to change notification settings - Fork 217
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
OS_cp cleanup #238
Comments
Imported from trac issue 215. Created by cdknight on 2019-05-23T15:32:53, last modified: 2019-06-05T13:20:39 |
Trac comment by cdknight on 2019-05-23 15:52:40: spun off from #116 I've created a branch "TRAC-93+215_cp_rename" that is a straw-man set of fixes for discussion. |
Trac comment by jhageman on 2019-05-23 16:38:29: Confirmed the shared next-gen implementation does it correctly, so this only applies to last-gen. |
Trac comment by jphickey on 2019-05-23 16:45:12: Subscribing. Yes, I came to comment that the "ng" OSAL already does it this way. |
Trac comment by jhageman on 2019-05-29 14:50:17: Is there still interest in fixing the last gen implementation? We could review [changeset:b10d3e2] if it's useful. Recommendation is to use next gen and close as "wontfix" if not. |
Trac comment by jhageman on 2019-06-05 13:20:39: CCB 6/5/2019 - scope is last-gen, deprecating use so closing as "wontfix" |
Looking at OS_cp, which does a system call to the "cp" command, this is really ugly, not realtime-friendly, and probably a security risk (if you could somehow inject a ';' into the system call, which shouldn't be difficult...Doesn't look like OS_TranslatePath catches this.)
I suggest re-writing OS_cp to do an open, a loop of read/write, and close (with the appropriate checks to ensure errors are handled.)
Also looks like OS_TranslatePath does the check for NULL and length, so the OS_{cp|mv|remove|...} should not do the same.
The text was updated successfully, but these errors were encountered: