List:MySQL++« Previous MessageNext Message »
From:Russell Robinson Date:October 29 2008 12:37pm
Subject:Re: Patch for ssqls null blob deficiency
View as plain text  
Hi,

On Wed, 29 Oct 2008 02:08:11 pm Warren Young wrote:
> On Oct 27, 2008, at 10:53 PM, Russell Robinson wrote:
> > I guess you (and/or others) need to tell me whether the changes to
> > type_info
> > are correct, but given my reasoning is correct together with the
> > stepwise
> > resolution of this problem I think they must be.
>
> I still worry that it might overreach, but dtest doesn't complain,
> soo...

Could be an overreach.  I can only work within the small view I have for the 
library and my instinct for debugging code.

It's a small set of changes, just in some "awkward" code.

It would be nice if you felt easier about the changes.  You know a lot more 
about the library than I do, so your uneasiness, makes me uneasy too.

Is there anyone else around who can offer an opinion about whether the changes 
make sense or there's a better way?

> > I've found the right way to do this is to convert it using
> > "stream2string".
>
> This is a rather heavy-handed fix.  It makes an extra copy of the
> BLOB, unnecessarily.

Ooops - you're right.

>
> The real problem is that you were constructing the SQLBuffer with a
> pointer-to-char, which calls the ctor taking std::string.  If you
> construct a std::string from char*, it only copies up to the first
> null character; bad ju-ju with BLOBs.  There must have been a null
> character at the fifth byte in your test data.

Yes!  It took me hours to figure out why the binary wasn't being quoted.  

Reason: it *was* being quoted, but the data was junk!

>
> You can fix the bug without the extra buffer copy by using SQLBuffer's
> other ctor, which takes char* and a length value.  svn diff -
> r2386:2387 to see the fix.

Thanks for that.  I must admit my focus was on getting the code to work, not 
efficiency.

And, I tried using that ctor, but couldn't figure out the right length value 
for when the value was to be null.  I saw the following code for all the 
other types, and thought "voila".

> Thanks for doing the legwork on this!

Thanks for fixing the inefficiency!

-- 
Russell Robinson (russellr@stripped)
Thread
Patch for ssqls null blob deficiencyRussell Robinson27 Oct
  • Re: Patch for ssqls null blob deficiencyRussell Robinson27 Oct
  • Re: Patch for ssqls null blob deficiencyWarren Young27 Oct
    • Re: Patch for ssqls null blob deficiencyRussell Robinson27 Oct
      • Re: Patch for ssqls null blob deficiencyWarren Young28 Oct
        • Re: Patch for ssqls null blob deficiencyRussell Robinson28 Oct
        • Re: Patch for ssqls null blob deficiencyRussell Robinson28 Oct
          • Re: Patch for ssqls null blob deficiencyRussell Robinson28 Oct
            • Re: Patch for ssqls null blob deficiencyRussell Robinson28 Oct
              • Re: Patch for ssqls null blob deficiencyWarren Young28 Oct
                • Re: Patch for ssqls null blob deficiencyRussell Robinson28 Oct
                  • Re: Patch for ssqls null blob deficiencyRussell Robinson28 Oct
                    • Re: Patch for ssqls null blob deficiencyWarren Young29 Oct
                      • Re: Patch for ssqls null blob deficiencyRussell Robinson29 Oct
                        • Re: Patch for ssqls null blob deficiencyWarren Young29 Oct
                          • Re: Patch for ssqls null blob deficiencyRussell Robinson29 Oct