-
Notifications
You must be signed in to change notification settings - Fork 12
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
sys: use strstr() to detect installed packages #8
base: master
Are you sure you want to change the base?
sys: use strstr() to detect installed packages #8
Conversation
Don't rely on sscanf() doesn't care about unused suffixes of the status string. Use strstr() instead to make sure only actually installed packages are returned. Suggested-by: Eric Fahlgren <[email protected]> Reported-by: Eric Fahlgren <[email protected]> Fixes: openwrt#6 Signed-off-by: Daniel Golle <[email protected]>
Looks good to me! |
Good |
@@ -233,8 +233,7 @@ rpc_sys_packagelist(struct ubus_context *ctx, struct ubus_object *obj, | |||
break; | |||
case 'S': | |||
if (is_field("Status", line)) | |||
if (sscanf(line, "Status: install %63s installed", tmp) == 1) | |||
installed = true; | |||
installed = !!strstr(line, " installed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to ensure: this line can't be not installed
, right?.
It would be nice to place a comment with a sample string Status: install 3 installed
. But that's a minor thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, the other possible strings are half-installed
or not-installed
with a dash. (Although I've never seen the half- version in the wild, not sure what it does.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, to be completely correct, here's the list (I only cared about the *installed
ones above):
static const enum_map_t pkg_state_status_map[] = {
{SS_NOT_INSTALLED, "not-installed"},
{SS_UNPACKED, "unpacked"},
{SS_HALF_CONFIGURED, "half-configured"},
{SS_INSTALLED, "installed"},
{SS_HALF_INSTALLED, "half-installed"},
{SS_CONFIG_FILES, "config-files"},
{SS_POST_INST_FAILED, "post-inst-failed"},
{SS_REMOVAL_FAILED, "removal-failed"}
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
This should now be committed on the 24.10 branch (and earlier), as #10 on main will supersede it. |
ping |
Don't rely on sscanf() doesn't care about unused suffixes of the status string. Use strstr() instead to make sure only actually installed packages are returned.
Suggested-by: @efahl
Reported-by: @efahl
Fixes: #6