On 07/11, Denys Vlasenko wrote:
>
> I propose to save this information in core dump, as a new note
> in note segment.
Denys, I am in no position to discuss whether we need this change or not,
format, etc. I'll only try to comment the code.
And please do not use the attachments ;)
> +static void fill_files_note(struct memelfnote *note)
> +{
> + struct vm_area_struct *vma;
> + struct file *file;
> + unsigned count, word_count, size, remaining;
> + long *data;
> + long *start_end_ofs;
> + char *name;
> +
> + count = 0;
> + for (vma = current->mm->mmap; vma != NULL; vma = vma->vm_next) {
> + file = vma->vm_file;
> + if (!file)
> + continue;
> + count++;
> + if (count >= MAX_FILE_NOTE_SIZE / 64) /* paranoia check */
> + goto err;
Why this check? If count is huge, then...
> + size = count * 64;
> + word_count = 2 + 3 * count;
> + alloc:
> + if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
> + goto err;
we should detect this case before the first alloc?
> + size = (size + PAGE_SIZE - 1) & (-PAGE_SIZE);
Well, I'd suggest PAGE_MASK instead of -PAGE_SIZE. Better yet,
size = round_up(size, PAGE_SIZE);
> + if (remaining == 0) {
> + try_new_size:
> + vfree(data);
> + size = size * 5 / 4;
> + goto alloc;
> + }
> + filename = d_path(&file->f_path, name, remaining);
> + if (IS_ERR(filename)) {
> + if (PTR_ERR(filename) == -ENAMETOOLONG)
> + goto try_new_size;
This looks like unnecessary complication to me, or I missed something.
d_path(..., buflen) should handle the "buflen == 0" case correctly, so
afacics you can remove the "if (remaining == 0)" block and move this
free-and-goto-alloc code under the -ENAMETOOLONG check.
> + while ((remaining--, *name++ = *filename++) != '\0')
> + continue;
Well, perhaps this is just me... but this looks a bit too complex
to me ;) I won't insist, but
do
remaining--;
while ((*name++ = *filename++));
looks more understandable, imho.
Or even
/* d_path() fills the end of the buffer */
remaining = name - filename;
strcpy(name, filename);
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
