[Kaul <mykaul@xxxxxxxxx>]

1. (line 180) tagsize is an int, probably should be an unsigned int (it's a length of a tag). Not sure what happens if a negative value is fetched, but doesn't look right.
2. (line 187) it verifies tvb_offset_exists(tvb, offset) - but then fetches values from 4 and 8 bytes after offset. Probably should be changed to tvb_offset_exists(tvb, offset + 8) ?

3. (line 194) After fetching tag name as a uint32, it also fetches it as a string, char by char, by printing (as %c) the result of 4 tvb_get_guint8() calls. Strange, but legit. I want, however, to change it to use value_string.

4. The dissector recursively calls dissect_daap_one_tag(), I'm not sure what will happen if a huge specially malformed segment (it's over a HTTP response) will be dissected by Wireshark. I suspect we should limit the recursion level, not only when there's nothing deeper to dissect.

5. (line 169) It seems to perform the dissection only if(tree), which means it might not dissect it if the tree does not exist. Probably should dissect it anyway, no?
6. (line 164) It performs col_append_fstr() with 8 bytes fetched from the tvb - without verifying those 8 bytes are available first. This fetching is also redundant (happens again in lines 190, 191).

I'd be happy to send patches to {some|all} the above, if people agree with {some|all} my observations.

Hopefully, it'll help me understand why my iTunes server doesn't work ;-)

Y.