Re: [repro-devel] repro patches
Thanks Ruslan,
Some additional comments:
1. Data patch - I don't think the Data::operator>(const Data& rhs)
change makes sense - it should not return true if lhs and rhs are equal.
2. I've committed the fixes to HttpBase - thanks.
3. I have the same question as Byron regarding the use of DB_DBT_MALLOC
for BerkleyDb.cxx - just want to understand this fix better.
Also - I would love to see repro as windows service, and I like the idea
of a new table to eliminate command line parameters. : )
Thanks,
Scott
BTW: You have "Scott Godin" as your display name for the repro-devel
list - it's a little misleading - can you please change this?
> -----Original Message-----
> From: repro-devel-bounces@xxxxxxxxxxxxxxx [mailto:repro-devel-
> bounces@xxxxxxxxxxxxxxx] On Behalf Of Byron Campen
> Sent: Tuesday, October 16, 2007 9:48 AM
> To: Ruslan Radvansky
> Cc: Scott Godin
> Subject: Re: [repro-devel] repro patches
>
> There actually isn't a double-free in ProcessorChain, since we
> take
> ownership from the auto_ptr passed in addProcessor() (see line 46).
>
> I see what you're trying to avoid in the patches to Data, but
> operator== is pulling an interesting trick to ensure that both are
> the same length.
>
> *snip*
> bool
> resip::operator==(const Data& lhs, const char* rhs)
> {
> assert(rhs); // .dlb. not consistent with constructor
> if (memcmp(lhs.mBuf, rhs, lhs.mSize) != 0)
> {
> return false;
> }
> else
> {
> // make sure the string terminates at size
> return (rhs[lhs.mSize] == 0);
> }
> }
> *snip*
>
> Let us consider the three pertinent possibilities:
> - lhs (Data) is longer than rhs (char*) : The memcmp will fail,
> because it will see a null (ie, the end) in rhs, and won't see one in
> lhs.
> - lhs (Data) is shorter than rhs (char*) : If the memcmp fails,
> things are ok. If the memcmp passes, we will then look for a null
> (ie, the end) in rhs, and not find it, because it occurs later on.
> This will cause us to return false.
> - lhs (Data) and rhs (char*) are the same length : If the memcmp
> passes, the two match, and the null check will verify this.
>
> Now, I should note that we are assuming that lhs doesn't contain
> any
> nulls. (Data is geared such that it _can_ contain nulls in the middle
> of the string, since it can hold raw data.) If there was a null in
> just the right place (ie, in the same position as the end of rhs),
> the memcmp _could_ continue into uninitialized memory. Is this
> something we're willing to put up with?
>
> The DB_THREAD thread-safety fix to BerkeleyDb is a good catch,
> and I
> think I understand the essence of the other fixes (I assume if we
> don't specify any of DB_DBT_MALLOC, DB_DBT_REALLOC, or
> DB_DBT_USERMEM, it will use static memory to hold the data, which
> won't be threadsafe?).
>
> The fixes to HttpBase look pretty straightforward, although I
> know
> squat about the Windows socket API.
>
> Best regards,
> Byron Campen
>
> > Hello Scott,
> >
> > Sorry for long delay. I was send previous mail to wrong address.
> >
> > proposed patches:
> > Data.cxx - some kiddy bugs in compare operators.
> > HttpBase.hxx - selfincluded.
> > HttpBase.cxx - in Win32 handles opened by socket() must be
> > closed by closesocket(). I'm unaware about cygwin.
> > BerkleyDb.cxx - free-threading.
> > ProcessorChain.cxx - double free memory (see comment)
> >
> > will be:
> > in *Store race condition still exist (findfirst/findnext). Do not
> > told me about "Only WebAdmin have write access to *Store", I'll
> > write extension to repro for internal use.
> >
> > May you interest it:
> > I plan to:
> > 1) run repro as Windows service.
> > 2) Because BerkleyDb and MySql have two licenses (for commercial
> > use)i want to add really free DB frontend for repro. I think about
> > firebird.
> >
> > I want to discuss - adding new table to db frontend. This table will
> > be contain parameters currently acessed only by commandline.
> >
> > --
> > Best regards,
> > Ruslan mailto:prostoruslan@xxxxxxxxx
> > <ProcessorChain.cxx.diff>
> > <BerkleyDb.diff>
> > <Data.diff>
> > <HttpBase.cxx.diff>
> > <HttpBase.hxx.diff>
> > _______________________________________________
> > repro-devel mailing list
> > repro-devel@xxxxxxxxxxxxxxx
> > https://list.resiprocate.org/mailman/listinfo/repro-devel
>
> _______________________________________________
> repro-devel mailing list
> repro-devel@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/repro-devel