Re: [libvirt] [PATCH] virAsprintf cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Guido Günther <agx@xxxxxxxxxxx> wrote:
> please find a possible asprintf vs. virAsprintf cleanup attached. There
> were several occations where the result of a failed asprintf was used.

Nice work.
ACK.

> diff --git a/.x-sc_prohibit_asprintf b/.x-sc_prohibit_asprintf
> new file mode 100644
> index 0000000..614c238
> --- /dev/null
> +++ b/.x-sc_prohibit_asprintf
> @@ -0,0 +1,3 @@
> +^gnulib/
> +^po/
> +ChangeLog
> diff --git a/Makefile.maint b/Makefile.maint
> index 29c50fc..095e3d6 100644
> --- a/Makefile.maint
> +++ b/Makefile.maint
> @@ -105,6 +105,12 @@ sc_prohibit_strcmp:
>  	  { echo '$(ME): use STREQ in place of the above uses of str''cmp' \
>  		1>&2; exit 1; } || :
>
> +# Use virAsprintf rather than a''sprintf since *strp is undefined on error.
> +sc_prohibit_asprintf:
> +	@grep -nE '\<a''sprintf\>' $$($(VC_LIST_EXCEPT)) && \
> +	  { echo '$(ME): do not use a''sprintf but virAsprintf'\
> +		1>&2; exit 1; } || :

This looks fine, but it might be a little clearer to use a one-character
range in the regexp in place of the pair of adjacent single quotes.
And a couple other minor changes:

# Use virAsprintf rather than a'sprintf since *strp is undefined on error.
sc_prohibit_asprintf:
	@grep -nE '\<[a]sprintf\>' $$($(VC_LIST_EXCEPT)) && \
	  { echo '$(ME): use virAsprintf, not a'sprintf 1>&2; exit 1; } || :

...

> diff --git a/src/util.c b/src/util.c
...
> @@ -1170,7 +1167,7 @@ virParseNumber(const char **str)
>  /**
>   * virAsprintf
>   *
> - * like asprintf but makes sure *strp == NULL on failure
> + * like libc's_asprintf but makes sure *strp == NULL on failure

s/libc/glibc/

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]