john-dev - Re: Coding Style

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 25 Jul 2015 23:43:11 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: Coding Style

Thanks for magnum's great advice.

>> 3.2.4 When type conversion, add a space before '*'.
>>
>> E.g:
>>
>> hostSalt = (cl_uint *) mem_alloc(SALT_BUFFER_SIZE);
>
> I personally disagree. We should mandate NOT adding a space, or just
> be liberal about it. Actually I always drop the space after the paren too:
>
>  hostSalt = (cl_uint*)mem_alloc(SALT_BUFFER_SIZE);
>
> I think we should allow (not require) a space after the paren, and
> either allow or require that there's space before the '*'.

There are many codes either add or not add a space before the '*'.
You can find those codes by:
find -name "*.[ch]" | xargs grep -n "char \*)"
find -name "*.[ch]" | xargs grep -n "char\*)"

So just be liberal about it ?

>> 9.3 Things to avoid when using macros:
>
> Same here. These are good advices but I think they have no place in
> our coding style.

Do you think we should allow the following macros ?

1) macros that affect control flow:

        #define FOO(x)                                  \
                do {                                    \
                        if (blah(x) < 0)                \
                                return -EBUGGERED;      \
                } while(0)

is a _very_ bad idea.  It looks like a function call but exits the "calling"
function; don't break the internal parsers of those who will read the code.

There are some macros in john have return. Such as:

scrypt_fmt.c:274

#define H0(s) \
        char *cp = strrchr(s,'#39;)+40; \
        int i = cp-s; \
        return i > 0 ? H((s), i) & 0xF : 0

>> Chapter 10: The inline disease
>>
>> A reasonable rule of thumb is to not put inline at functions that have
>> more than 3 lines of code in them. An exception to this rule are the
>> cases where a parameter is known to be a compiletime constant,
>> and as a result of this constantness you *know* the compiler will be
>> able to optimize most of your function away at compile time.
>
> While this is probably reasonable advice, I think we don't need it in
> our code style.

Should we add 'inline' to a function which has 97 lines ?

unicode.c:184
inline int utf8_to_utf16(UTF16 *target, unsigned int len, const UTF8
*source,
                         unsigned int sourceLen)
{
        ...
}

Thanks,

Kai

Content of type "text/html" skipped

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.