On Sat, Feb 14, 2004 at 02:23:46PM -0600, Edwin Calo wrote:
> Thanks for the feedback and I made some changes that I think
> will guarantee the loop will always terminate.
Just let me rephrase that: When I looked at your code the first time,
I couldn't immediately decide whether the loop terminates or not, and
I don't think that your code has changed anything regarding that, so
I will work with your original patch, not the second one.
> (Before I was just looking at the buff_remaining...)
When I first looked at the code, I saw that offset was sometimes being
increased, sometimes being decreased. Now that I have taken another
look at it, I can see that the decrease is always undone. Also, offset
is increased at the very beginning of the loop and buff_remaining is
calculated at the end of loop, so it should terminate.
...
> #include <epan/strutil.h>
> #include "packet-rpc.h"
> #include "plugins/plugin_api.h"
These aren't needed.
...
> buff_remaining = tvb_length_remaining (tvb, offset);
...
> buff_remaining = tvb_length_remaining (tvb, offset);
> /* Used to print the initial buff remaining */
> /* if (check_col (pinfo->cinfo, COL_INFO)) { col_append_fstr (pinfo->cinfo, COL_INFO, " BuffRemainig: %d", buff_remaining ); } */
Looks like there is one call to buff_remaining too much
> proto_tree_add_string (tree,hf_postgresql_string,tvb, offset,counter, tvb_get_ptr(tvb, offset, counter));
> string = tvb_get_ptr (tvb, offset, counter);
> /* Forcing end to string */
> string[counter]='\0';
I'm not really sure that this does what you are trying to accomplish:
a) According to doc/README.tvbuff, tvb_get_ptr might create a copy of the
data, so running tvb_get_ptr twice wouldn't acomplish anything. In
case no copy is made, things are even worse, because you are changing
the original data. Also, it seems that you are writing one byte
*past* the end of the buffer.
b) gcc complains about "assignment of read-only loaction"
An example of how this can be done can be found in packet-http.c,
ft the end of unction is_http_request_or_reply (request_method).
Hmm, maybe just using tvb_memdup should be ok (and fixing that off
by 1 problem).
Ciao
Jörg
PS: epan/tvbuff.h contains an even stricter warning on the use of
tvb_get_ptr.
--
Joerg Mayer <jmayer@xxxxxxxxx>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
