These patches look good to me, but it would be nice to get rid of uglies like:
> +/*
> + * Written by Dave Hansen <dave.han...@intel.com>
> + *
> + * run like this:
> + pid=31390; BDIR="$(cat /proc/$pid/smaps | grep -B1 2097152 | head -1 | awk
> -F- '{print $1}')"; ./mpx-dig $pid 0x$BDIR
> +
> +NOTE:
> + assumes that the only 2097152-kb VMA is the bounds dir
> + */
(weird vertical alignment)
> +long nr_incore(void *ptr, unsigned long size_bytes)
> +{
> + int i;
> + long ret = 0;
> + long vec_len = size_bytes / PAGE_SIZE;
> + unsigned char *vec = malloc(vec_len);
> + if (!vec)
(missing newline)
> + mpx_dig_abort();
> +
> + int incore_ret = mincore(ptr, size_bytes, vec);
(weird non-kernel-style definition.)
> +char buf[100];
> +int open_proc(int pid, char *file)
(missing newline)
> +{
> + int fd;
> + sprintf(&buf[0], "/proc/%d/%s", pid, file);
(ditto, also unsafe looking sprintf())
> +void __dave_abort(int line)
> +{
> + perror("abort");
> + printf("abort @ %d\n", line);
> + mpx_dig_abort();
> +}
> +#define dave_mpx_dig_abort() __dave_abort(__LINE__);
maybe s/dave// ? ;-)
> +//This mincore stuff works, but the bounds tables are not
> +//sparse enough to make it worthwhile
bad comment style.
> + unsigned char incore_vec[MPX_BOUNDS_TABLE_SIZE_BYTES/ PAGE_SIZE];
> + int incore_ret = mincore(bt_buf, MPX_BOUNDS_TABLE_SIZE_BYTES,
> &incore_vec[0]);
stray whitespace.
> + //if (!incore_vec[page_nr])
> + // continue;
ugly.
> + if (this_bt_entry_for_vaddr > 0x00007fffffffffffUL) {
> + this_bt_entry_for_vaddr |= 0xffff800000000000;
> + }
unnecessary curly braces.
> + // Each bounds directory entry controls 1MB of
> + // virtual address space. This variable is the
> + // virtual address in the process of the
> + // beginning of the area controlled by this
> + // bounds_dir.
ugly.
> + unsigned long bd_for_vaddr = bd_index * (1UL<<20);
> + //printf("%s() at bd index: %lx for vaddr: %lx\n", __func__,
> bd_index, bd_for_vaddr);
> + //printf("dir entry[%4ld @ %p]\n", bd_index,
> bounds_dir_global+i);
Ditto.
> + int nr_entries = dump_table(bounds_dir_entry, bd_for_vaddr,
> bounds_dir_global+bd_offset_bytes+i);
> + total_entries += nr_entries;
> + continue;
> + printf("dir entry[%4ld @ %p]: 0x%lx %6d entries total this buf:
> %7d bd_for_vaddrs: 0x%lx -> 0x%lx\n",
> + bd_index, buf+i,
> + bounds_dir_entry, nr_entries, total_entries,
> bd_for_vaddr, bd_for_vaddr + (1UL<<20));
hm?
> + // there shouldn't practically be short reads of /proc/$pid/mem
> +
> + incore_ret = mincore(dig_bounds_dir_ptr, buffer_size_bytes, &vec[0]);
stray space.
> + //if (this_entries)
> + // printf("entries this loop: %d total: %d (bufs skipped:
> %d)\n", this_entries, total_entries, bufs_skipped);
You know the drill!
> +#ifdef MPX_DIG_REMOTE
> +int main(int argc, char **argv)
> +{
> + int err;
> + char *c;
hm...
etc. - ad nauseum. Would be nice to tidy this all up a bit.
Thanks,
Ingo
