[pacman-dev] [PATCH] Prevent warnings with unchecked asprintf and FORTIFY_SOURCE

Allan McRae allan at archlinux.org
Mon Jun 28 00:41:04 EDT 2010


On 28/06/10 11:56, Andres P wrote:
> Instead of throwing away the return value, make a single function handle
> errors.
>
> Signed-off-by: Andres P<aepd87 at gmail.com>
> ---
>
> I strongly disagree with making this void, but until all these void
> funcions get a rework then I guess this is better than outright ignoring.

Then make it return an int.  The functions calling this can be adjusted 
later, but at least adding the error message is a step in the right 
direction.

>   src/pacman/util.c |   33 ++++++++++++++++++++++-----------
>   1 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index de1b162..1b2f451 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -44,6 +44,17 @@
>   #include "conf.h"
>   #include "callback.h"
>
> +/* error handling in a single fn please */

Seriously, this is one of the my useless pieces of function 
documentation I have ever seen.

> +void vw_asprintf(char **str, const char *fmt, ...)

Why "vw_"?  We have several other similar wrappers called pm_<foo> in 
util.c already, so stick with that naming for consistency.

Also, add the function proto to util.h.

> +{
> +	va_list args;
> +	va_start(args, fmt);
> +	if(vasprintf(str, fmt, args) != 0) {

...   oh, I see you flag this issue in the next email

> +		pm_fprintf(stderr, PM_LOG_ERROR,  _("failed to allocate string\n"));
> +		/* goto halt */
> +	}
> +	va_end(args);
> +}
>
>   int trans_init(pmtransflag_t flags)
>   {
> @@ -530,10 +541,10 @@ void display_targets(const alpm_list_t *pkgs, int install)
>   			double mbsize = 0.0;
>   			mbsize = alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
>
> -			asprintf(&str, "%s-%s [%.2f MB]", alpm_pkg_get_name(pkg),
> +			vw_asprintf(&str, "%s-%s [%.2f MB]", alpm_pkg_get_name(pkg),
>   					alpm_pkg_get_version(pkg), mbsize);
<snip>

As an aside, it was obvious that this was something I was working on. 
So duplicating the work is a waste of both of our time.   On that note, 
do not bother resubmitting with my comments addressed as it would make 
the patch almost identical to what I have locally.

Allan



More information about the pacman-dev mailing list