Ticket #362 (closed defect: fixed)
Incorrect usage of shell_escape() and shell_unescape() functions
| Reported by: | andrew_b | Owned by: | slavazanko |
|---|---|---|---|
| Priority: | blocker | Milestone: | 4.7 |
| Component: | mc-core | Version: | master |
| Keywords: | commited-master | Cc: | |
| Blocked By: | Blocking: | ||
| Branch state: | Votes for changeset: |
Description
As shell_escape() as shell_unescape() functions returns new allocated string. That string must be free when uneeded. But there are many incorrect usages of shell_escape() and shell_unescape():
src/complete.c:217
src/complete.c:505
src/complete.c:561
vfs/fish.c:545
vfs/fish.c:548
vfs/fish.c:564
src/file.c:2122
src/file.c:2123
src/command.c:70
src/complete.c:90
src/complete.c:482
Maybe related to #356
Change History
comment:2 in reply to: ↑ 1 Changed 15 years ago by andrew_b
Replying to slyfox:
src/complete.c:561
Does not look like leak (original string is g_free'd)
Yes, you're right.
vfs/fish.c:545
vfs/fish.c:548
vfs/fish.c:564
src/complete.c:90
src/complete.c:482
Yes. Inplace semantic of unescape_buf function was broken after revert.
src/command.c:70
Yep. examine_cd could stricter have (const char *) proto
path = shell_unescape(path);
Since inplace semantic of shell_unescape() is currently broken, such usage is wrong.
Will we implement the inplace modifying of input buffer in the shell_unescape() function?
comment:3 follow-up: ↓ 4 Changed 15 years ago by slavazanko
- Keywords review added
- Status changed from new to accepted
- Version changed from 4.6.2 to master
- Owner set to slavazanko
Will we implement the inplace modifying of input buffer in the shell_unescape() function?
IMHO, not needed.
created branch 362_fix_escape_unescape_alloc (parent: master)
Initial commit: changeset:47a49cc96b7bf859f3b3331cc603598fda42c528
Review, please.
And #356 now closed - because that same problem (but raise in different ways).
comment:4 in reply to: ↑ 3 Changed 15 years ago by andrew_b
Replying to slavazanko:
Initial commit: changeset:47a49cc96b7bf859f3b3331cc603598fda42c528
Review, please.
This commit introduced new bug: pointer to freed memory is used in command_completion_function(). Fixed.
In filename_completion_function(), the tilde_expand() was used incorrect. Fixed.
Alsow small optimization have beed performed.
Yes. Looks like those leaks were introduced by commit:0450daf56672a75df543c8222353c8042de8c7a1
Does not look like leak (original string is g_free'd)
Yes. Inplace semantic of unescape_buf function was broken after revert.
Yep. examine_cd could stricter have (const char *) proto