List:Commits« Previous MessageNext Message »
From:Tatjana Azundris Nurnberg Date:November 27 2007 12:00pm
Subject:Re: bk commit into 5.0 tree (tnurnberg:1.2557) BUG#31177
View as plain text  
Sergei Golubchik <serg@stripped> writes:


  Hi!

  Thanks for looking into this!

> this part of my_getopt.c of 5.1 is not necessary for this bugfix.

  That is certainly correct.  If you do look at 5.0 / 5.1 however,
  you'll find that they're already different from the version I
  copied from (in that they have basic error reporting and such,
  for a start); I was simply hoping to prevent the trees from
  further drift.  Either is fine with me.  Will mark in CS comments
  as suggested.



> I don't like this null_reporter idea. Let init_one_value issue warnings
> normally for every value adjustment.
> Of course there should be no warnings, that is, you need to fix all
> options where default values are outside of min..max range.

  Can do ... done.



>> +  */
>> +  if (chk && (option_limits->block_size > 1) && (*chk ==
> in))
>> +  {
>> +    struct my_option tmp_limits;
>> +    memcpy(&tmp_limits, option_limits, sizeof(struct my_option));
>> +    tmp_limits.block_size= 1;
>
> why ???

  Because I was too reluctant to change the signature of
  getopt_ull_limit_value().  I've done that now; it's much
  prettier.



> I don't think you're supposed to do any checks or issue any warnings in
> ::update(), they belong to ::check().

  You know, that looked funky to me too, but it wasn't my idea.
  That's where *existing* sys_var_thd_ulong::update() both checks
  *and* warns.  It's also where all the other methods call
  getopt_ull_limit_value() at least, which checks and adjusts
  (and in pre-patch 5.0 already warns to the log, just not to
  the client yet), not to mention some other checks and adjustments
  (--max-variable= etc.).  So there's a lot of precedent there.
  Obviously, you can still be right and the existing code wrong,
  but I think that's out of the scope of this bug.

  Will attach updated patch once the tests finish.

  thanks and regards,
  Tatiana


-- 
Tatjana A. Nurnberg, Bugs Bunny * MySQL AB, http://www.mysql.de/ * EST
IRC: Azundris * Skype: azundris * MySQL-SIP: 4550 * eMail: azundris@stripped
.de: Firmensitz: MySQL GmbH, Radlkoferstr. 2, D-81373 Muenchen
     Geschaeftsfuehrer: Hans von Bell, Kaj Arnoe - HRB Muenchen 162140
Thread
bk commit into 5.0 tree (tnurnberg:1.2557) BUG#31177Tatjana A Nuernberg26 Nov
  • Re: bk commit into 5.0 tree (tnurnberg:1.2557) BUG#31177Sergei Golubchik26 Nov
    • Re: bk commit into 5.0 tree (tnurnberg:1.2557) BUG#31177Tatjana Azundris Nurnberg27 Nov