On Tue, Feb 07, 2012 at 11:42:00PM +0100, Davidlohr Bueso wrote:
> +struct lock {
> + struct list_head locks;
> +
> + char *cmdname;
> + pid_t pid;
> + char *path;
> + char *type;
> + long start;
> + long end;
off_t rather than long
> + int mandatory;
> + char *size;
> +};
> +
> +/*
> + * Return a PID's command name
> + */
> +static char *get_cmdname(pid_t pid)
> +{
> + FILE *fp;
> + char path[PATH_MAX], *cmd = NULL;
> +
> + sprintf(path, "/proc/%d/status", pid);
> + if (!(fp = fopen(path, "r")))
> + return NULL;
/proc/<pid>/comm contains only the command name, you don't have to
parse anything.
> + /* just need the first line */
> + if (!fgets(path, sizeof(path), fp))
> + goto out;
> +
> + (void)strtok_r(path, "\t", &cmd);
> + cmd[strlen(cmd) - 1] = '\0';
> +out:
> + fclose(fp);
> + return xstrdup(cmd);
> +}
> +
> +/*
> + * Associate the device's mountpoint for a filename
> + */
> +static char *get_fallback_filename(size_t dev)
> +{
> + char buf[PATH_MAX], *ret = NULL;
> + char *key = NULL, *tmp = NULL, *tok = NULL;
> + int i;
> + struct stat sb;
> + FILE *fp;
> +
> + if (!(fp = fopen(_PATH_PROC_MOUNTS, "r")))
> + return NULL;
> +
> + while (fgets(buf, sizeof(buf), fp)) {
> + for (i = 0, tmp = buf; ; tmp = NULL, i++) {
> + if (!(tok = strtok_r(tmp, " ", &key)))
> + break;
> +
> + if (i == 1)
> + if (!stat(tok, &sb) && dev == sb.st_dev) {
- you call stat() for all mountpoint, it's not too elegant...
- stat->st_dev is completely useless on btrfs, because it
returns a different devno than kernel uses in /proc.
See https://bugzilla.redhat.com/show_bug.cgi?id=711881
Try "flock -x /mnt lslk", where /mnt is btrfs mountpoint, the
tools is not able to convert /proc/locks information to mountpoint
path...
It seems that more robust solution is to use /proc/self/mountinfo
where in 3rd column is maj:min and this devno is the same as devno in
/proc/locks.
Note that /proc/locks uses %02x:%02x format, mountinfo uses decimal
numbers.
Please, use scanf() for parsing rather than strtok(), especially for
simple things like "maj:min:ino".
> + ret = xstrdup(tok);
> + goto out;
> + }
> + }
> + }
> +
> +out:
> + fclose(fp);
> + return ret;
> +}
> +
> +/*
> + * Return the absolute path of a file from
> + * a given inode number (and its size)
> + */
> +static char *get_filename_sz(long inode, pid_t pid, size_t *size)
> +{
> + struct stat sb;
> + struct dirent *dp;
> + DIR *dirp;
> + size_t len;
> + char path[PATH_MAX], sym[PATH_MAX], *ret = NULL;
> +
> + *size = 0;
> + memset(path, 0, sizeof(path));
> + memset(sym, 0, sizeof(sym));
> +
> + /*
> + * We know the pid so we don't have to
> + * iterate the *entire* filesystem searching
> + * for the damn file.
> + */
> + sprintf(path, "/proc/%d/fd/", pid);
> + if (!(dirp = opendir(path)))
> + return NULL;
> +
> + if ((len = strlen(path)) >= (sizeof(path) - 2))
> + goto out;
> +
> + while ((dp = readdir(dirp))) {
> + if (!strcmp(dp->d_name, ".") ||
> + !strcmp(dp->d_name, ".."))
> + continue;
> +
> + /* care only for numerical descriptors */
> + if (!strtol(dp->d_name, (char **) NULL, 10))
> + continue;
> +
> + (void) strcpy(&path[len], dp->d_name);
> + if (!stat(path, &sb) && inode != sb.st_ino)
> + continue;
> +
> + if ((len = readlink(path, sym, sizeof(path))) < 1)
> + goto out;
Please use statat() and readlinkat(), in lib/at.c are portable
versions.
> + *size = sb.st_size;
sym[len] = '\0';
> + ret = xstrdup(sym);
> + break;
> + }
> +out:
> + closedir(dirp);
> + return ret;
> +}
> +
> +/*
> + * Return the inode number from a string
> + */
> +static long get_dev_inode(char *str, dev_t *dev)
> +{
> + char *tmp = NULL, *tok = NULL, *key = NULL;
> + int i, maj = 0, min = 0;
> + long inum = 0;
> +
> + for (i = 0, tmp = str; ; tmp = NULL, i++) {
> + if (!(tok = strtok_r(tmp, ":", &key)))
> + break;
> +
> + /*
> + * traditional format '<major>:<minor>:<inode>' - note that this
sscanf(str, "%02x:02x:%ul", ....), this whole function will have 5
lines... :-)
> + * might change and the kernel would export '<name>:<inode>'.
> + */
> + switch (i) {
> + case 0:
> + maj = strtol(tok, (char **) NULL, 16);
> + break;
> + case 1:
> + min = strtol(tok, (char **) NULL, 16);
> + break;
> + case 2:
> + inum = strtol_or_err(tok, _("failed to parse inode number"));
> + default:
> + break;
> + }
> + }
> +
> + *dev = (dev_t) makedev(maj, min);
> + return inum;
> +}
> +
> +static int get_local_locks(struct list_head *locks)
> +{
> + int i;
> + long inode;
> + FILE *fp;
> + char buf[PATH_MAX], *key = NULL, *tmp = NULL, *tok = NULL;
> + size_t sz;
> + struct lock *l;
> + dev_t dev;
> + pid_t p;
> +
> + if (!(fp = fopen(_PATH_PROC_LOCKS, "r")))
> + return 0;
> +
> + while (fgets(buf, sizeof(buf), fp)) {
> + l = xcalloc(1, sizeof(*l));
> + INIT_LIST_HEAD(&l->locks);
> +
> + for (i = 0, tmp = buf; ; tmp = NULL, i++) {
> + if (!(tok = strtok_r(tmp, " ", &key)))
> + break;
for (i = 0, tok = strtok(buf, " ");
tok = strtok(NULL, " ");
i++)
key and tmp are unnecessary
> +
> + /*
> + * /proc/locks as *exactly* 8 "blocks" of text
> + * separated by ' ' - check <kernel>/fs/locks.c
> + */
> + switch (i) {
> + case 0: /* not intereted! */
> + case 1: /* not intereted! */
> + break;
> +
> + case 2: /* is this a mandatory lock? other values are advisory or noinode */
> + l->mandatory = *tok == 'M' ? TRUE : FALSE;
> + break;
> + case 3: /* lock type */
> + l->type = xstrdup(tok);
> + break;
> +
> + case 4: /* PID */
> + /*
> + * If user passed a pid we filter it later when adding
> + * to the list, no need to worry now.
> + */
> + l->pid = strtol_or_err(tok, _("failed to parse pid"));
> + l->cmdname = get_cmdname(l->pid);
> + break;
> +
> + case 5: /* device major:minor and inode number */
> + inode = get_dev_inode(tok, &dev);
> + break;
> +
> + case 6: /* start */
> + l->start = !strcmp(tok, "EOF") ? 0 :
> + strtol_or_err(tok, _("failed to parse start"));
> + break;
> +
> + case 7: /* end */
> + /* replace '\n' character */
> + tok[strlen(tok)-1] = '\0';
> + l->end = !strcmp(tok, "EOF") ? 0 :
> + strtol_or_err(tok, _("failed to parse end"));
> + break;
> + default:
> + break;
> + }
> +
> + l->path = get_filename_sz(inode, l->pid, &sz);
> + /* probably no permission to peek into l->pid's path */
> + if (!l->path)
> + l->path = get_fallback_filename(dev);
> +
> + /* avoid leaking */
> + l->size = xstrdup(size_to_human_string(SIZE_SUFFIX_1LETTER, sz));
I see leak, size_to_human_string() returns allocated string.
[...]
> +int main(int argc, char *argv[])
> +{
> + int c, tt_flags = 0;
> + struct list_head locks;
> + enum {
> + RAW_OPTION = CHAR_MAX + 1,
> + NOHEADINGS_OPTION
> + };
> + static const struct option long_opts[] = {
> + { "pid", required_argument, NULL, 'p' },
> + { "help", no_argument, NULL, 'h' },
> + { "version", no_argument, NULL, 'V' },
> + { "noheadings", no_argument, NULL, NOHEADINGS_OPTION },
> + { "raw", no_argument, NULL, RAW_OPTION },
why not -r and -n for raw and noheadings?
> + { NULL, 0, NULL, 0 }
> + };
[...]
> +
> + return EXIT_SUCCESS;
> +}
always EXIT_SUCCESS? Don't be so optimistic :-)
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
