Re: [Nautilus-list] Cache getpwuid results

[Darin Adler <darin bentspoon com>]

On Tuesday, August 14, 2001, at 06:13  PM, Alex Larsson wrote:

> @@ -582,10 +586,13 @@ nautilus_file_denies_access_permission (
>  				        GnomeVFSFilePermissions group_permission,
>  				        GnomeVFSFilePermissions other_permission)
>  {
> -	uid_t user_id;
> -	struct passwd *password_info;
> -	gid_t supplementary_groups[NGROUPS_MAX];
> -	int num_supplementary_groups;
> +	struct timeval now;
> +	static struct timeval cached_time = {0,0};
> +	static struct passwd *password_info;
> +	static uid_t user_id;
> +	static gid_t primary_group;
> +	static gid_t supplementary_groups[NGROUPS_MAX];
> +	static int num_supplementary_groups;
>  	int i;
>
>  	g_assert (NAUTILUS_IS_FILE (file));
> @@ -602,9 +609,23 @@ nautilus_file_denies_access_permission (
>  		return FALSE;
>  	}
>
> -	/* Check the user. */
> -	user_id = geteuid ();
> +	/* This is called often. Cache the user information for five 
> minutes */
> +	
> +	gettimeofday (&now, NULL);
> +	
> +	if (cached_time.tv_sec == 0 ||
> +	    (now.tv_sec - cached_time.tv_sec) > GETPWUID_CACHE_TIME) {
> +		cached_time = now;
> +		user_id = geteuid ();
> +		/* No need to free result of getpwuid. */
> +		password_info = getpwuid (user_id);
> +		if (password_info)
> +			primary_group = password_info->pw_gid;
> +		num_supplementary_groups = getgroups (NGROUPS_MAX, 
> supplementary_groups);
> +	}

Even though it would make things a bit less elegant, it would make this 
lot cleaner if the caching and getting the user information was done in a 
separate function. Would you be willing to refactor this into a separate 
function?

Also, I think you need to set primary_group to 0 when password_info is 
NULL, because otherwise you might remember an old primary group from the 
last time you checked, 5 minutes ago. (Not that this will ever really come 
up.)

In addition, I normally like to steer clear of minor shortcuts like 
assuming that the tv_sec of a real time will never be 0. A separate flag 
to tell you if you've ever gotten the user ID information would do the 
trick without requiring any subtlety, but you may find that last elegant.

Otherwise, the patch looks great, and I'd encourage you to get a corrected 
version in soon.

    -- Darin