Hi Jan,
Jan Safranek wrote:
>> --- a/include/libcgroup.h
>> +++ b/include/libcgroup.h
>> @@ -285,6 +285,19 @@ int cgroup_get_task_begin(char *cgroup, char
>> *controller, void **handle,
>> pid_t *pid);
>>
>> /**
>> + * Get process data (euid, egid, and process name) from
>> /proc/<pid>status
>> + * file.
>> + * @param pid: The process id
>> + * @param euid: If this is NULL, the euid is not taken
>> + * @param egid: If this is NULL, the egid is not taken
>> + * @param procname: If this is NULL, the process name is not taken
>> + * @param size_procname: The buffer size of procname
>> + * @return 0 on success, > 0 on error.
>> + */
>> +int cgroup_get_procdata_from_status(pid_t pid, uid_t *euid, gid_t *egid,
>> + char *procname, size_t size_procname);
>> +
>
> I am not sure whether this function should be public. It has nothing to
> do with cgroups, it's helper function of tools/daemons. What do others say?
Not only tools/daemon but also cgclassify command.
There is almost same code in tools/cgclassify.c and I think it is better
to merge one function.
I will change this function to cgroup_get_uid_gid_from_procfs() without
getting process name.
>> +/**
>> * Read the next task value
>> * @handle: The handle used for iterating
>> * @pid: The variable where the value will be stored
>> diff --git a/src/api.c b/src/api.c
>> index 9da3ebb..0739642 100644
>> --- a/src/api.c
>> +++ b/src/api.c
>> @@ -2477,3 +2477,58 @@ int cgroup_get_task_begin(char *cgroup, char
>> *controller, void **handle,
>>
>> return ret;
>> }
>> +
>> +int cgroup_get_procdata_from_status(pid_t pid, uid_t *euid, gid_t *egid,
>> + char *procname, size_t size_procname)
>> +{
>> + FILE *f;
>> + char path[FILENAME_MAX];
>> + char buf[4092];
>> + uid_t ruid, suid, fsuid;
>> + gid_t rgid, sgid, fsgid;
>> + bool find_euid = false;
>> + bool find_egid = false;
>> + bool find_procname = false;
>> +
>> + if (euid)
>> + find_euid = true;
>> + if (egid)
>> + find_egid = true;
>> + if (procname)
>> + find_procname = true;
>> +
>> + sprintf(path, "/proc/%d/status", pid);
>> + f = fopen(path, "r");
>> + if (!f)
>> + return 1;
>> +
>> + memset(buf, '\0', sizeof(buf));
>
> Is the memset necessary?
Right, I will remove it.
>> + while (fgets(buf, sizeof(buf), f)) {
>> + if (!strncmp(buf, "Uid:", 4) && find_euid) {
>> + sscanf((buf + 5), "%d%d%d%d", &ruid, euid,
>> + &suid, &fsuid);
>
> How stable is this /proc file format? Should there be a check that
> sscanf succeeded?
You are right. The file format in /proc FS is not stable, and it may
be changed in future. As you said, the check of sscanf() should be
checked. I will add it.
>> + cgroup_dbg("Scanned proc values are %d %d %d %d\n",
>> + ruid, *euid, suid, fsuid);
>> + find_euid = false;
>> + } else if (!strncmp(buf, "Gid:", 4) && find_egid) {
>
> dtto, check sscanf return value?
Yes, I will add it also.
>> + sscanf((buf + 5), "%d%d%d%d", &rgid, egid,
>> + &sgid, &fsgid);
>> + cgroup_dbg("Scanned proc values are %d %d %d %d\n",
>> + rgid, *egid, sgid, fsgid);
>> + find_egid = false;
>> + } else if (!strncmp(buf, "Name:", 5) && find_procname &&
>> + (strlen(buf + 6) < size_procname)) {
>
> If the size_procname is not big enough, the function returns magic value
> '1', without any other indication what's wrong. Should we introduce
> something like 'ECG NOT ENOUGH MEMORY'? And return amount of needed
> memory in size_procname (which then needs to be size_t*)??? Or use 'char
> procname[PATH_MAX] instead?
I will separate the feature for getting process name from this function,
and I think the separated function is as following:
char *get_procname_from_procfs(pid_t pid);
This function allocates memory for a process name, writes a process name onto
it, and returns the pointer. So a caller should free the memory when unusing it.
If fails, this function returns NULL.
>> + sscanf((buf + 6), "%s", procname);
>> + find_procname = false;
>> + }
>> + if (!find_euid && !find_egid && !find_procname) {
>> + fclose(f);
>> + return 0;
>> + }
>> + memset(buf, '\0', sizeof(buf));
I will remove this memset() also.
Thanks
Ken'ichi Ohmichi
------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, &
iPhoneDevCamp as they present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
