[apparmor] [PATCH 2/5] Abstract out the construction of proc path generation more.

[John Johansen]

On 07/12/2011 12:59 PM, Steve Beattie wrote:
> On Tue, Jul 12, 2011 at 11:49:02AM -0700, John Johansen wrote:
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> ---
>>  libraries/libapparmor/src/kernel_interface.c |   21 ++++++++++++++-------
>>  1 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/libraries/libapparmor/src/kernel_interface.c b/libraries/libapparmor/src/kernel_interface.c
>> index 4a48799..f48792f 100644
>> --- a/libraries/libapparmor/src/kernel_interface.c
>> +++ b/libraries/libapparmor/src/kernel_interface.c
>> @@ -46,10 +46,18 @@ static inline pid_t aa_gettid(void)
>>  #endif
>>  }
>>  
>> -static int setprocattr(const char *path, const char *buf, int len)
>> +static char *procattr_path(pid_t pid, const char *attr)
>> +{
>> +	char *path = NULL;
>> +	if (asprintf(&path, "/proc/%d/current/%s", pid, attr) > 0)
> 
> ITYM "/proc/%d/attr/%d" here. I don't know if that makes you inclined to
> change the attr variable name, since it's a directory of attr(ibutes).
yep, and I think attr is fine for the variable

> (A unit test or two would be kinda nice and might have let you catch
> that; see how tst_aalogmisc.c is handled for an example.)
> 
yep, but it would only work for the procattr_path fn and not for the
interface as a whole.  I was planning on adding some regression tests
for the interface.

> Otherwise patch is okay.
> 
>> +		return path;
>> +	return NULL;
>> +}
>> +
>> +static int setprocattr(const char *attr, const char *buf, int len)
>>  {
>>  	int rc = -1;
>> -	int fd, ret, ctlerr = 0;
>> +	int fd, ret;
>>  	char *ctl = NULL;
>>  	pid_t tid = aa_gettid();
>>  
>> @@ -58,10 +66,9 @@ static int setprocattr(const char *path, const char *buf, int len)
>>  		goto out;
>>  	}
>>  
>> -	ctlerr = asprintf(&ctl, path, tid);
>> -	if (ctlerr < 0) {
>> +	ctl = procattr_path(tid, attr);
>> +	if (!ctl)
>>  		goto out;
>> -	}
>>  
>>  	fd = open(ctl, O_WRONLY);
>>  	if (fd == -1) {
>> @@ -113,7 +120,7 @@ int aa_change_hat(const char *subprofile, unsigned long token)
>>  		goto out;
>>  	}
>>  
>> -	rc = setprocattr("/proc/%d/attr/current", buf, len);
>> +	rc = setprocattr("current", buf, len);
>>  out:
>>  	if (buf) {
>>  		/* clear local copy of magic token before freeing */
>> @@ -144,7 +151,7 @@ int aa_change_profile(const char *profile)
>>  	if (len < 0)
>>  		return -1;
>>  
>> -	rc = setprocattr("/proc/%d/attr/current", buf, len);
>> +	rc = setprocattr("current", buf, len);
>>  
>>  	free(buf);
>>  	return rc;
>> -- 
>> 1.7.5.4
>>
>>
>> -- 
>> AppArmor mailing list
>> AppArmor at lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
> 
> 
>