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@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