On Wed, Jul 24, 2024 at 03:52:01PM -0700, Andrii Nakryiko wrote:
SNIP
> +static int freader_get_page(struct freader *r, u64 file_off)
> +{
> + pgoff_t pg_off = file_off >> PAGE_SHIFT;
> +
> + freader_put_page(r);
> +
> + r->page = find_get_page(r->mapping, pg_off);
> + if (!r->page)
> + return -EFAULT; /* page not mapped */
> +
> + r->page_addr = kmap_local_page(r->page);
> + r->file_off = file_off & PAGE_MASK;
> +
> + return 0;
> +}
> +
> +static const void *freader_fetch(struct freader *r, u64 file_off, size_t sz)
> +{
> + int err;
> +
> + /* provided internal temporary buffer should be sized correctly */
> + if (WARN_ON(r->buf && sz > r->buf_sz)) {
> + r->err = -E2BIG;
> + return NULL;
> + }
what's the benefit of having err, would it be easier just to return
error pointer like ERR_PTR(-E2BIG)
SNIP
> +static void freader_cleanup(struct freader *r)
> +{
> + freader_put_page(r);
> +}
> +
> /*
> * Parse build id from the note segment. This logic can be shared between
> * 32-bit and 64-bit system, because Elf32_Nhdr and Elf64_Nhdr are
> * identical.
> */
> -static int parse_build_id_buf(unsigned char *build_id,
> - __u32 *size,
> - const void *note_start,
> - Elf32_Word note_size)
> +static int parse_build_id_buf(struct freader *r,
> + unsigned char *build_id, __u32 *size,
> + u64 note_offs, Elf32_Word note_size)
> {
> - Elf32_Word note_offs = 0, new_offs;
> + const char note_name[] = "GNU";
could be static ?
SNIP
> int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size)
> {
> - return parse_build_id_buf(build_id, NULL, buf, buf_size);
> + struct freader r;
> +
> + freader_init_from_mem(&r, buf, buf_size);
> +
> + return parse_build_id_buf(&r, build_id, NULL, 0, buf_size);
could use a coment in here why freader_cleanup is not needed
jirka
> }
>
> #if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID) || IS_ENABLED(CONFIG_VMCORE_INFO)
> --
> 2.43.0
>
>
