Wireshark · Wireshark-dev: Re: [Wireshark-dev] A set of patches to allow a pcap-ng file to be piped into wireshark

[Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>]

On Sat, May 12, 2012 at 11:39:11PM -0700, Richard Sharpe wrote:
> Attached is a set of patches that seems to do the trick.
> [...]
> It would be useful if people could review them ...

This patch is too big for me, it'd be great if this patch could be splited.
But some notes:

1/ hdr, hdr_size in cap_pipe_open_live() are assigned but actually never read.

2/ You don't need to do put break; after return;

3/ This look incorrect:
     pcap_opts->hdr.u.pcapng.block_ptr = 
        g_malloc(pcap_opts->hdr.u.pcapng.blk_hdr.block_total_length - sizeof(pcapng_block_header_t) - 4);
     pcap_opts->cap_pipe_bytes_to_read = 
                (pcap_opts->hdr.u.pcapng.blk_hdr.block_total_length - sizeof(pcapng_block_header_t) + 3) & ~0x3;

    pcap_opts->cap_pipe_bytes_to_read is larger than allocated buffer, is it ok?

4/ g_malloc returns NULL only when size is 0.
   You can either use g_try_malloc() or remove if-null-check.

5/ Can you use G_STRFUNC from glib instad of __func__?

6/ cap_pipe_read_pcapng_header() after allocating block_ptr should it be freed in error paths?
   (some goto error2?)


Btw. I haven't tried it, but I'm afraid that your code doesn't work properly with multiple pipes/sources.
(I think it needs some work with IDB-blocks (merge all/current into one?), and later fixing interface_id in other blocks)
It's still ok for me, but we need to signal error when user requested it.

Cheers,
 Kuba.