lkml.org 
[lkml]   [2010]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Patch 18/21] Generate the data sections for ELF Core
于 2010年12月14日 18:22, Suzuki K. Poulose 写道:
...
> + /*
> + * Read from the vma segments
> + * a. verify if the *fpos is within a phdr
> + * b. Use access_process_vm() to get data page by page
> + * c. copy_to_user into user buffer
> + */


This kind of comments is useless, "code tells you how, comments tell you why."


> +
> + while (buflen) {
> + size_t bufsz, offset, bytes;
> + char *readbuf;
> + struct elf_phdr *phdr = get_pos_elfphdr(cp, *fpos);
> +
> + if (!phdr)
> + break;
> +
> + bufsz = (buflen> PAGE_SIZE) ? PAGE_SIZE : buflen;
> + readbuf = kmalloc(bufsz, GFP_KERNEL);
> + if (!readbuf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + offset = *fpos - phdr->p_offset;
> + bytes = access_process_vm(cp->task, (phdr->p_vaddr + offset),
> + readbuf, bufsz, 0);
> + if (!bytes) {
> + ret = -EIO;
> + goto out;


Why you don't kfree(readbuf) here?

> + }
> + if (copy_to_user(buffer, readbuf, bytes)) {
> + ret = -EFAULT;
> + kfree(readbuf);
> + goto out;
> + } else
> + acc += bytes;
> +
> + kfree(readbuf);
> + buflen -= bytes;
> + buffer += bytes;
> + *fpos += bytes;
> + }
> +
> + /* Fill extnum section header if present */
> + if (buflen&&
> + elf_hdr->e_shoff&&
> + (*fpos>= elf_hdr->e_shoff)&&
> + (*fpos< (elf_hdr->e_shoff + sizeof(struct elf_shdr)))) {


This indention seems ugly.

> +
> + off_t offset = *fpos - elf_hdr->e_shoff;

Are you sure it is 'off_t' not 'loff_t'?

> + size_t shdrsz = sizeof(struct elf_shdr) - offset;
> +
> + shdrsz = (buflen< shdrsz) ? buflen : shdrsz;
> + if (copy_to_user(buffer, ((char *)cp->shdr) + offset, shdrsz)) {
> + ret = -EFAULT;
> + goto out;
> + } else {
> + acc += shdrsz;
> + buflen -= shdrsz;
> + buffer += shdrsz;
> + }
> + }
>
> +done:
> + ret = acc;
> out:
> return ret;
> }

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-12-15 08:23    [W:0.231 / U:1.612 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site