[libvirt] [PATCH] virt-aa-helper: remove replace_string and use virStringReplace instead
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/security/virt-aa-helper.c | 76 +++++-------------------------------------- 1 file changed, 9 insertions(+), 67 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 7eeb4ef..537e89d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -148,62 +148,6 @@ vah_info(const char *str) } /* - * Replace @oldstr in @orig with @repstr - * @len is number of bytes allocated for @orig. Assumes @orig, @oldstr and - * @repstr are null terminated - */ -static int -replace_string(char *orig, const size_t len, const char *oldstr, - const char *repstr) -{ - int idx; - char *pos = NULL; - char *tmp = NULL; - - if ((pos = strstr(orig, oldstr)) == NULL) { - vah_error(NULL, 0, _("could not find replacement string")); - return -1; - } - - if (VIR_ALLOC_N_QUIET(tmp, len) < 0) { - vah_error(NULL, 0, _("could not allocate memory for string")); - return -1; - } - tmp[0] = '\0'; - - idx = abs(pos - orig); - - /* copy everything up to oldstr */ - strncat(tmp, orig, idx); - - /* add the replacement string */ - if (strlen(tmp) + strlen(repstr) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strcat(tmp, repstr); - - /* add everything after oldstr */ - if (strlen(tmp) + strlen(orig) - (idx + strlen(oldstr)) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strncat(tmp, orig + idx + strlen(oldstr), - strlen(orig) - (idx + strlen(oldstr))); - - if (virStrcpy(orig, tmp, len) == NULL) { - vah_error(NULL, 0, _("error replacing string")); - VIR_FREE(tmp); - return -1; - } - VIR_FREE(tmp); - - return 0; -} - -/* * run an apparmor_parser command */ static int @@ -340,6 +284,7 @@ create_profile(const char *profile, const char *profile_name, char *pcontent = NULL; char *replace_name = NULL; char *replace_files = NULL; + char *tmp = NULL; const char *template_name = "\nprofile LIBVIRT_TEMPLATE"; const char *template_end = "\n}"; int tlen, plen; @@ -412,19 +357,16 @@ create_profile(const char *profile, const char *profile_name, goto clean_replace; } - if (VIR_ALLOC_N_QUIET(pcontent, plen) < 0) { - vah_error(NULL, 0, _("could not allocate memory for profile")); - goto clean_replace; - } - pcontent[0] = '\0'; - strcpy(pcontent, tcontent); - - if (replace_string(pcontent, plen, template_name, replace_name) < 0) + if (!(pcontent = virStringReplace(tcontent, template_name, replace_name))) goto clean_all; - if ((virtType != VIR_DOMAIN_VIRT_LXC) && - replace_string(pcontent, plen, template_end, replace_files) < 0) - goto clean_all; + if (virtType != VIR_DOMAIN_VIRT_LXC) { + if (!(tmp = virStringReplace(pcontent, template_end, replace_files))) + goto clean_all; + VIR_FREE(pcontent); + pcontent = tmp; + tmp = NULL; + } /* write the file */ if ((fd = open(profile, O_CREAT | O_EXCL | O_WRONLY, 0644)) == -1) { -- 2.8.2
On Fri, 2016-05-13 at 10:55 +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/security/virt-aa-helper.c | 76 +++++-------------------------------------- 1 file changed, 9 insertions(+), 67 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 7eeb4ef..537e89d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -148,62 +148,6 @@ vah_info(const char *str) } /* - * Replace @oldstr in @orig with @repstr - * @len is number of bytes allocated for @orig. Assumes @orig, @oldstr and - * @repstr are null terminated - */ -static int -replace_string(char *orig, const size_t len, const char *oldstr, - const char *repstr) -{ - int idx; - char *pos = NULL; - char *tmp = NULL; - - if ((pos = strstr(orig, oldstr)) == NULL) { - vah_error(NULL, 0, _("could not find replacement string")); - return -1; - } - - if (VIR_ALLOC_N_QUIET(tmp, len) < 0) { - vah_error(NULL, 0, _("could not allocate memory for string")); - return -1; - } - tmp[0] = '\0'; - - idx = abs(pos - orig); - - /* copy everything up to oldstr */ - strncat(tmp, orig, idx); - - /* add the replacement string */ - if (strlen(tmp) + strlen(repstr) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strcat(tmp, repstr); - - /* add everything after oldstr */ - if (strlen(tmp) + strlen(orig) - (idx + strlen(oldstr)) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strncat(tmp, orig + idx + strlen(oldstr), - strlen(orig) - (idx + strlen(oldstr))); - - if (virStrcpy(orig, tmp, len) == NULL) { - vah_error(NULL, 0, _("error replacing string")); - VIR_FREE(tmp); - return -1; - } - VIR_FREE(tmp); - - return 0; -} - -/* * run an apparmor_parser command */ static int @@ -340,6 +284,7 @@ create_profile(const char *profile, const char *profile_name, char *pcontent = NULL; char *replace_name = NULL; char *replace_files = NULL; + char *tmp = NULL; const char *template_name = "\nprofile LIBVIRT_TEMPLATE"; const char *template_end = "\n}"; int tlen, plen; @@ -412,19 +357,16 @@ create_profile(const char *profile, const char *profile_name, goto clean_replace; } - if (VIR_ALLOC_N_QUIET(pcontent, plen) < 0) { - vah_error(NULL, 0, _("could not allocate memory for profile")); - goto clean_replace; - } - pcontent[0] = '\0'; - strcpy(pcontent, tcontent); - - if (replace_string(pcontent, plen, template_name, replace_name) < 0) + if (!(pcontent = virStringReplace(tcontent, template_name, replace_name))) goto clean_all; - if ((virtType != VIR_DOMAIN_VIRT_LXC) && - replace_string(pcontent, plen, template_end, replace_files) < 0) - goto clean_all; + if (virtType != VIR_DOMAIN_VIRT_LXC) { + if (!(tmp = virStringReplace(pcontent, template_end, replace_files))) + goto clean_all; + VIR_FREE(pcontent); + pcontent = tmp; + tmp = NULL; + } /* write the file */ if ((fd = open(profile, O_CREAT | O_EXCL | O_WRONLY, 0644)) == -1) {
ACK As a side note, the use of cleanup labels could probably be simplified a lot here, if you feel like doing that in a separate patch... -- Andrea Bolognani Software Engineer - Virtualization Team
On Fri, May 13, 2016 at 05:10:15PM +0200, Andrea Bolognani wrote:
On Fri, 2016-05-13 at 10:55 +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/security/virt-aa-helper.c | 76 +++++-------------------------------------- 1 file changed, 9 insertions(+), 67 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 7eeb4ef..537e89d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -148,62 +148,6 @@ vah_info(const char *str) } /* - * Replace @oldstr in @orig with @repstr - * @len is number of bytes allocated for @orig. Assumes @orig, @oldstr and - * @repstr are null terminated - */ -static int -replace_string(char *orig, const size_t len, const char *oldstr, - const char *repstr) -{ - int idx; - char *pos = NULL; - char *tmp = NULL; - - if ((pos = strstr(orig, oldstr)) == NULL) { - vah_error(NULL, 0, _("could not find replacement string")); - return -1; - } - - if (VIR_ALLOC_N_QUIET(tmp, len) < 0) { - vah_error(NULL, 0, _("could not allocate memory for string")); - return -1; - } - tmp[0] = '\0'; - - idx = abs(pos - orig); - - /* copy everything up to oldstr */ - strncat(tmp, orig, idx); - - /* add the replacement string */ - if (strlen(tmp) + strlen(repstr) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strcat(tmp, repstr); - - /* add everything after oldstr */ - if (strlen(tmp) + strlen(orig) - (idx + strlen(oldstr)) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strncat(tmp, orig + idx + strlen(oldstr), - strlen(orig) - (idx + strlen(oldstr))); - - if (virStrcpy(orig, tmp, len) == NULL) { - vah_error(NULL, 0, _("error replacing string")); - VIR_FREE(tmp); - return -1; - } - VIR_FREE(tmp); - - return 0; -} - -/* * run an apparmor_parser command */ static int @@ -340,6 +284,7 @@ create_profile(const char *profile, const char *profile_name, char *pcontent = NULL; char *replace_name = NULL; char *replace_files = NULL; + char *tmp = NULL; const char *template_name = "\nprofile LIBVIRT_TEMPLATE"; const char *template_end = "\n}"; int tlen, plen; @@ -412,19 +357,16 @@ create_profile(const char *profile, const char *profile_name, goto clean_replace; } - if (VIR_ALLOC_N_QUIET(pcontent, plen) < 0) { - vah_error(NULL, 0, _("could not allocate memory for profile")); - goto clean_replace; - } - pcontent[0] = '\0'; - strcpy(pcontent, tcontent); - - if (replace_string(pcontent, plen, template_name, replace_name) < 0) + if (!(pcontent = virStringReplace(tcontent, template_name, replace_name))) goto clean_all; - if ((virtType != VIR_DOMAIN_VIRT_LXC) && - replace_string(pcontent, plen, template_end, replace_files) < 0) - goto clean_all; + if (virtType != VIR_DOMAIN_VIRT_LXC) { + if (!(tmp = virStringReplace(pcontent, template_end, replace_files))) + goto clean_all; + VIR_FREE(pcontent); + pcontent = tmp; + tmp = NULL; + } /* write the file */ if ((fd = open(profile, O_CREAT | O_EXCL | O_WRONLY, 0644)) == -1) {
ACK
Thaks, pushed.
As a side note, the use of cleanup labels could probably be simplified a lot here, if you feel like doing that in a separate patch...
Yes, I've noticed that. This patch was mainly to fix clang build and Jan pointed out, that the replace_string() could be removed and replaced by virStringReplace().
participants (2)
-
Andrea Bolognani -
Pavel Hrdina