List:Internals« Previous MessageNext Message »
From:MARK CALLAGHAN Date:October 18 2009 7:35pm
Subject:Re: Coding style changes of 2009-06-26 now in the guidelines
View as plain text  
On Sun, Oct 18, 2009 at 3:34 AM, Michael Widenius <monty@stripped> wrote:
>
> Hi!
>
>>>>>> "MARK" == MARK CALLAGHAN <mdcallag@stripped> writes:
>
> MARK> On Sat, Oct 17, 2009 at 10:55 AM, Ingo Strüwing
> <Ingo.Struewing@stripped> wrote:
>>> Hi all,
>>>
>>> the coding style changes, agreed upon by the MySQL Server coding style
>>> government committee on 2009-06-26, is now part of the Coding Guidelines
>>> in the internals manual:
>>> http://forge.mysql.com/wiki/MySQL_Internals_Coding_Guidelines
>
> MARK> I am glad these have been published. I have comments and questions
> MARK> about bool and my_bool.
>
>>>>>
> MARK>     *  Functions should return zero on success, and non-zero
> on error,
> MARK> so you can do:
>>>>>
>
> MARK> Can this be extended to state that these functions should return int
> MARK> rather than bool/my_bool? Some functions today use bool/my_bool and
> MARK> return TRUE on error. I have to read the code for these to figure out
> MARK> whether TRUE is returned on error.
>
> Functions that only returns 0 or 1 should be bool or my_bool.

I am being pedantic, but functions that return TRUE or FALSE can be
bool/my_bool. A lot of MySQL source uses 0/1 in place of FALSE/TRUE
for bool/my_bool which makes the code harder to read.

I am not against using bool, I am against using it to return TRUE on
error. The libc model is for functions to return type int with 0 on
success and not 0 on error. I want the same to be done in MySQL.
Predicate functions (has_a(), is_a()) can return type bool.

> Normally you should assume that <> 0 is an error.

I want a higher standard than this as 'normally' allows for too many
exceptions. I don't want to remember those exceptions.

>
> MARK> ----------------------------
>
>>>>>
> MARK> bool exists only in C++. In C, you have to use my_bool (which is
> MARK> char); it has different cast rules than bool:
> MARK> int c= 256*2;
> MARK> bool a= c; /* a gets 'true' */
> MARK> my_bool b= c; /* b gets zero, i.e. 'false': BAD */
> MARK> my_bool b= test(c); /* b gets 'true': GOOD */
>>>>>
>
> MARK> Given the examples above, my_bool is completely broken, use of it will
> MARK> introduce bugs and should be removed from the code. Is that scheduled
> MARK> to be done?
>
> I personally like my_bool, as it gives me information 'at a glance' if
> a function only returns 0 or 1. As C doesn't have 'bool', we have to
> resort to use my_bool. Storing anything else than 0 or 1 in a my_bool
> is to regarded as bug in the cdoe that needs to be fixed.
>
> Fortunately some newer compilers gives a warning when you assign an
> int to a char and we will find errors like this in our build system
> and get them fixed.

I prefer to use a datatype that doesn't have this problem -- where
(my_bool) 256 == 0, (my_bool) 257 == 1.

New versions of gcc print a warning for this when -Wconversion is
used, older versions support -Wconversion but don't warn for this. But
MySQL doesn't even use -Wall, much less -Wconversion and there are 301
distinct warnings for -Wconversion from code in the sql directory with
MySQL 5.1.38.

-- 
Mark Callaghan
mdcallag@stripped
Thread
Coding style changes of 2009-06-26 now in the guidelinesIngo Strüwing17 Oct
  • Re: Coding style changes of 2009-06-26 now in the guidelinesMARK CALLAGHAN17 Oct
    • Re: Coding style changes of 2009-06-26 now in the guidelinesMichael Widenius18 Oct
      • Re: Coding style changes of 2009-06-26 now in the guidelinesMARK CALLAGHAN18 Oct
        • Re: Coding style changes of 2009-06-26 now in the guidelinesMichael Widenius18 Oct
          • Re: Coding style changes of 2009-06-26 now in the guidelinesJay Pipes18 Oct
          • Re: Coding style changes of 2009-06-26 now in the guidelinesMARK CALLAGHAN18 Oct
            • Re: Coding style changes of 2009-06-26 now in the guidelinesMichael Widenius19 Oct
    • Re: Coding style changes of 2009-06-26 now in the guidelinesIngo Strüwing19 Oct
      • Re: Coding style changes of 2009-06-26 now in the guidelinesMats Kindahl19 Oct
        • RE: Coding style changes of 2009-06-26 now in the guidelinesVladislav Vaintroub19 Oct
          • Re: Coding style changes of 2009-06-26 now in the guidelinesMats Kindahl19 Oct
            • RE: Coding style changes of 2009-06-26 now in the guidelinesVladislav Vaintroub19 Oct
            • Re: Coding style changes of 2009-06-26 now in the guidelinesRoy Lyseng19 Oct
          • RE: Coding style changes of 2009-06-26 now in the guidelinesMichael Widenius21 Oct
      • Re: Coding style changes of 2009-06-26 now in the guidelinesMARK CALLAGHAN19 Oct
        • Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Ingo Strüwing19 Oct
          • re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Michael Widenius21 Oct
            • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Ingo Strüwing21 Oct
              • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Michael Widenius21 Oct
                • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in the guidelines]MARK CALLAGHAN21 Oct
                  • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in the guidelines]Michael Widenius21 Oct
                  • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Konstantin Osipov23 Oct
                    • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Jay Pipes23 Oct
                • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Ingo Strüwing22 Oct
                  • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Michael Widenius22 Oct
                    • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Ingo Strüwing22 Oct
                      • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in theguidelines]Michael Widenius23 Oct
                    • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in the guidelines]MARK CALLAGHAN23 Oct
                      • Re: Style proposal [Re: Coding style changes of 2009-06-26 now in the guidelines]Michael Widenius24 Oct
        • Re: Coding style changes of 2009-06-26 now in the guidelinesMichael Widenius21 Oct
          • Re: Coding style changes of 2009-06-26 now in the guidelinesMARK CALLAGHAN21 Oct
            • Re: Coding style changes of 2009-06-26 now in the guidelinesMichael Widenius21 Oct