Wireshark-bugs: [Wireshark-bugs] [Bug 1723] Enhancement of text2cap for parsing flexibility
Date: Tue, 21 Aug 2007 21:37:02 +0000 (GMT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1723


richardv@xxxxxxxxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |richardv@xxxxxxxxxxxxx
            Summary|Enhamcement of text2cap for |Enhancement of text2cap for
                   |parsing flexibility         |parsing flexibility




------- Comment #3 from richardv@xxxxxxxxxxxxx  2007-08-21 21:37 GMT -------
These two patches appear to be identical: is that deliberate?

Some comments on the patch:
* Generally it's not necessary to put your name against each change: this will
be obvious from the svn log anyway.

* declaring variables in mid-block ("int by_eol = 0;", for example, around line
990) isn't valid C89 and isn't supported by some of the compilers we support.
Please change this. In that particular case I suggest you just use
(token==T_EOL) in its place.

* Please could you add comments explaining what token_seq and pkt_lnstart do,
next to their declarations? Isn't token_seq the same as (state == INIT || state
== START_OF_LINE) anyway?

* Is adding new options necessary, or are your changes general enough to be the
default?

* If there is any possibility of you breaking this patch into several parts
which add different bits of functionality, I'd find it much easier to review
and would be consequently more likely to apply it...


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.