On Wed, Apr 05, 2017 at 04:36:26PM +0200, Martin Kletzander wrote:
> And use it in virFileRead*
>
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> src/util/virfile.c | 18 +++++++-----------
> src/util/virhostcpu.c | 4 ++--
> src/util/virstring.h | 8 ++++++++
> src/util/virsysfs.c | 2 ++
> 4 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index c0f448d3437d..cbfa3849d793 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3811,7 +3811,6 @@ int
> virFileReadValueInt(const char *path, int *value)
> {
> char *str = NULL;
> - char *endp = NULL;
>
> if (!virFileExists(path))
> return -2;
> @@ -3819,8 +3818,9 @@ virFileReadValueInt(const char *path, int *value)
> if (virFileReadAll(path, INT_STRLEN_BOUND(*value), &str) < 0)
> return -1;
>
> - if (virStrToLong_i(str, &endp, 10, value) < 0 ||
> - (endp && !c_isspace(*endp))) {
> + virStringTrimOptionalNewline(str);
> +
> + if (virStrToLong_i(str, NULL, 10, value) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Invalid integer value '%s' in file '%s'"),
> str, path);
> @@ -3847,7 +3847,6 @@ int
> virFileReadValueUint(const char *path, unsigned int *value)
> {
> char *str = NULL;
> - char *endp = NULL;
>
> if (!virFileExists(path))
> return -2;
> @@ -3855,8 +3854,9 @@ virFileReadValueUint(const char *path, unsigned int *value)
> if (virFileReadAll(path, INT_STRLEN_BOUND(*value), &str) < 0)
> return -1;
>
> - if (virStrToLong_uip(str, &endp, 10, value) < 0 ||
> - (endp && !c_isspace(*endp))) {
> + virStringTrimOptionalNewline(str);
> +
> + if (virStrToLong_uip(str, NULL, 10, value)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Invalid unsigned integer value '%s' in file '%s'"),
> str, path);
> @@ -3886,7 +3886,6 @@ virFileReadValueBitmap(const char *path,
> {
> char *buf = NULL;
> int ret = -1;
> - char *tmp = NULL;
>
> if (!virFileExists(path))
> return -2;
> @@ -3894,10 +3893,7 @@ virFileReadValueBitmap(const char *path,
> if (virFileReadAll(path, maxlen, &buf) < 0)
> goto cleanup;
>
> - /* trim optinoal newline at the end */
> - tmp = buf + strlen(buf) - 1;
> - if (*tmp == '\n')
> - *tmp = '\0';
> + virStringTrimOptionalNewline(buf);
>
> *value = virBitmapParseUnlimited(buf);
> if (!*value)
> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index 02b9fc8eb94f..a660e3f4dbe5 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -847,13 +847,13 @@ virHostCPUParseCountLinux(void)
> tmp = str;
> do {
> if (virStrToLong_i(tmp, &tmp, 10, &ret) < 0 ||
> - !strchr(",-\n", *tmp)) {
> + !strchr(",-", *tmp)) {
> virReportError(VIR_ERR_NO_SUPPORT,
> _("failed to parse %s"), str);
> ret = -1;
> goto cleanup;
> }
> - } while (*tmp++ != '\n');
> + } while (*tmp++ && *tmp);
> ret++;
>
> cleanup:
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index a5550e30d2e2..603650aa1588 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -288,4 +288,12 @@ bool virStringBufferIsPrintable(const uint8_t *buf, size_t buflen);
>
> char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
>
> +static inline void
> +virStringTrimOptionalNewline(char *str)
> +{
> + char *tmp = str + strlen(str) - 1;
> + if (*tmp == '\n')
> + *tmp = '\0';
> +}
Is there any other reason for using this instead of virTrimSpaces than just
being a bit faster? Because I think the performance gain in this case compared
to 1 iteration of the while loop is very small, thus if possible I would avoid
creating a function for it when there is virTrimSpaces (and I think
virSkipSpacesBackwards would be usable too).
Other than that, it makes perfect sense to me, ACK.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list