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.
