#34 lu_dispatch: Free tmp on failures
Opened 8 months ago by jhrozek. Modified 8 months ago
jhrozek/libuser free_tmp  into  master

file modified
+12 -3

@@ -980,7 +980,10 @@ 

  	case user_default:

  	case group_default:

  		/* Make sure we have both name and boolean here. */

- 		g_return_val_if_fail(sdata != NULL, FALSE);

+ 		if (sdata == NULL) {

+ 			free(tmp);

+ 			return FALSE;

+ 		}

  		/* Run the checks and preps. */

  		if (run_list(context, context->create_module_names,

  			    logic_and, id,

@@ -1059,7 +1062,10 @@ 

  	case user_setpass:

  	case group_setpass:

  		/* Make sure we have a valid password. */

- 		g_return_val_if_fail(sdata != NULL, FALSE);

+ 		if (sdata == NULL) {

+ 			free(tmp);

+ 			return FALSE;

+ 		}

  		/* no break: fall through */

  	case user_removepass:

  	case group_removepass:

@@ -1088,7 +1094,10 @@ 

  	case users_enumerate_by_group:

  	case groups_enumerate_by_user:

  		/* Make sure we have both name and ID here. */

- 		g_return_val_if_fail(sdata != NULL, FALSE);

+ 		if (sdata == NULL) {

+ 			free(tmp);

+ 			return FALSE;

+ 		}

  		if (id == users_enumerate_by_group)

  			ldata = convert_group_name_to_id(context, sdata,

  							 error);

Merges:
https://pagure.io/libuser/issue/23

This makes the code slightly less compact with using an explicit
condition instead of the g_return_val_if_fail() shorthand, but freeing
tmp on failures.

g_return_val_if_fail also logs a critical error; maybe use g_return_val_if_reached to preserve that?

(I’m honestly not all that sold on the g_*_if_fail style of error “handling”. But if it’s worth doing at all, it’s worth doing consistently.)

Metadata