[pacman-dev] [PATCH] pacman/upgrade: Check malloc() return value

[Sören Brinkmann]

On Sat, 2014-03-08 at 05:29PM +1000, Allan McRae wrote:
> On 08/03/14 17:07, Sören Brinkmann wrote:
> > On Sat, 2014-03-08 at 05:02PM +1000, Allan McRae wrote:
> >> On 07/03/14 16:24, Sören Brinkmann wrote:
> >>> Check the return value of malloc() before dereferencing the returned pointer.
> >>>
> >>> Signed-off-by: Sören Brinkmann <soeren.brinkmann at gmail.com>
> >>> ---
> >>>  src/pacman/upgrade.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
> >>> index 5416f6180b39..19aa17218ce4 100644
> >>> --- a/src/pacman/upgrade.c
> >>> +++ b/src/pacman/upgrade.c
> >>> @@ -51,6 +51,9 @@ int pacman_upgrade(alpm_list_t *targets)
> >>>  	 */
> >>>  	for(i = targets; i; i = alpm_list_next(i)) {
> >>>  		int *r = malloc(sizeof(int));
> >>> +		if(r == NULL) {
> >>> +			return 1;
> >>> +		}
> >>>  
> >>>  		if(strstr(i->data, "://")) {
> >>>  			char *str = alpm_fetch_pkgurl(config->handle, i->data);
> >>>
> >>
> >> Fine. Although if malloc of an int fails, I'm not sure we can do a lot!
> > 
> > Right, but bailing out gracefully is probably better than a segfault? I
> > do not really mind. If you don't want it, don't apply it. But not
> > checking the return value of malloc() is simply wrong, IMHO.
> > 
> > 	Sören
> > 
> 
> I want to add something like this in the check:
> 
>   pm_printf(ALPM_LOG_ERROR, _("memory exhausted\n"));
> 
> otherwise we just drop out with no notification to the user.  A segfault
> at least tells them something went wrong...  However, I'm not sure we
> can achieve that when we fail malloc for an int.

Hmm. I guess you're right. It is highly unlikely to ever hit this error
path. And if it ever really fails, chances are you are in really big
trouble. So, the whole thing is probably questionable. If the malloc
fails your whole system is probably failing and whether you check the
return value (with or without printing a message) may be irrelevant. I'd
assume your OS will already yell at you.
I just couldn't pass this malloc with missing error handling without
taking action.

Anyway, no problem to re-spin the patch and add the printf statement, if
you want that.

	Sören