List:MySQL++« Previous MessageNext Message »
From:Rick Gutleber Date:November 26 2008 9:41pm
Subject:Re: insertfrom( ) patch
View as plain text  
Warren Young wrote:
> Rick Gutleber wrote:
>>
>> there is a difference in the line-endings having to do with the jpeg 
>> examples in bmark.txt.
>
> I guess it's just cgi_jpeg you're talking about.  The DOS-style line 
> endings in its output are intentional.  HTTP uses DOS line endings in 
> its headers, and a CGI program basically just builds an HTTP reply, so 
> it has the same requirement.
>
> We do need to suppress the silly bmark.txt diffs somehow.  We can do 
> it by checking for -D, but I'd like to see if there a lighter hammer 
> we can use first.  Basically, someone needs to look at a *hex* diff to 
> see what's really going on at the byte level, to see if there's a way 
> to satisfy CGI without creating diffs when moving between platforms.


I definitely don't understand what's going on there.  I tried a trivial 
program that writes to standard out and ends lines with "\r\n" and it 
actually wrote the CR and the LF.


> On to patch comments:
>
> In dbdriver.h:
>
>>      /// \brief Executes the given query string
>>      ///
>>      /// Wraps \c mysql_real_query() in the MySQL C API.
>> +    ///
>> +    /// \sa escape_string(std::string*, const char*, size_t),
>> +    /// escape_string_no_conn(char*, const char*, size_t)
>>      bool execute(const char* qstr, size_t length)
>>              { return !mysql_real_query(&mysql_, qstr, length); }
>
> Why the \sa here?


Hrm.   Loose nut on the keyboard perhaps.


>
> NoTransaction should be moved to transaction.h.  I don't see that 
> anything in insertpolicy.h uses NoTransaction, so you don't need to 
> replace it with an #include for transaction.h or a forward declaration.

Good idea.  I'd naturally put it in insertpolicy.h because that's the 
thing that it's related to, but


>
> In query.h:
>
>> +#include "insertpolicy.h"
>
> Is there a reason you're not including this within the Query public 
> section, to add these classes to its definition?  I thought we'd 
> agreed that these classes don't need to be at the top level of the 
> mysqlpp namespace.


You're right.  We'd discussed that a while ago, but I forgot about it.


> Later in query.h:
>
>> +        for (Iter it = first; it != last; it++ ) {
>
> Always preincrement iterators.  The difference doesn't matter for 
> containers with simple pointer-like iterators -- vector, string, etc. 
> -- as the compiler can optimize away the difference in increment forms 
> just as it does for integers.  For more complex containers -- list, 
> deque, set... -- postincrementing can be an efficiency sap, because 
> compilers can't see that preincrementing is functionally identical in 
> a for loop like this, so it can do away with the temporaries 
> postincrementing requires.  The iterators are too complex for it to 
> make such a leap.  I believe this is covered in one of Meyers' 
> Effective C++ or STL books.


Noted.  I recall struggling with this issue with own class library years 
ago.


>
> Speaking of iterators, have you tested the patch with an associative 
> container?


I tried it with a set, but not anything else.  I guess you'd need a 
different version of the insertfrom( ) method to use a map or anything 
that is composed of pairs or something similar...  unless there's 
something I'm missing.


> And speaking of testing, what platforms have you tried this on?


Just Red Hat Enterprise Linux 3 with GCC 3.2.3.  That's all I have 
convenient access to.  Actually, I'm going to have access to a RHEL 5 
box soon, but not now.

>
> Other than that... http://snipurl.com/6svzs


http://tinyurl.com/57exr6


>
> I need to sneak 3.0.8 out soon, to take care of the VC++ Connection 
> errors and maybe one or two other things.  That done, I can apply your 
> patch to kick off the v3.1 process.
>
> You don't need to resubmit the patch.  You can just tell me what to do 
> about these when I apply the patch.  But if you want, you can. :)


I'll leave that to you.  Thanks.


>
> Also, is the anti-asplode patch coming separately?
>
 
I set that aside to submit separately, so yes. 

Rick



Thread
insertfrom( ) patchRick Gutleber24 Nov
  • Re: insertfrom( ) patchWarren Young25 Nov
    • Re: insertfrom( ) patchRick Gutleber26 Nov
      • Re: insertfrom( ) patchWarren Young17 Feb
        • Re: insertfrom( ) patchRick Gutleber17 Feb
          • Re: insertfrom( ) patchWarren Young17 Feb