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]
Message-ID: <CABtNtWFJy2VjmuR5QLMkP8OxSggh0oUu3p+-XBntfhBMynTfZw@mail.gmail.com>
Date: Fri, 5 Jun 2015 14:28:23 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: Coding Style

> I think we should try the indent command you posted on some files in
> jumbo and see what "breakage" it causes - in other words, while fixing
> improper formatting does it also make formatting changes that we won't
> like?

magnum has run indent and astyle with two files.

https://github.com/magnumripper/JohnTheRipper/commit/feb8a1521f7e69642cc430970cc1714c14698466

First:
  indent -kr -i4 -ts4 -nlp -nbbo -ncs -l79 -lc79 -bad -il0
common-opencl.[ch]
and then:
  astyle --style=kr -t4 -U -H -xC79 -c -k3 -z2 common-opencl.[ch]

There are three "breakage":

1. open brace { should be on the previous line
-------------------------------------------------------------

original
----------

void f1()
{
#if CON1
        {
#if CON2
                while (condition) {
                        do_something();
                        do_something();
                }
#endif
        }
#endif
}

formatted
-------------

void f1()
{
#if CON1
        {
#if CON2
                while (condition)
                {
                        do_something();
                        do_something();
                }
#endif
        }
#endif
}

2. There are strings like: " supported with --platform " "syntax.\n"
-------------------------------------------------------------------------------------

original
----------

void f2()
{
        fprintf(stderr, "Only one OpenCL device"
                        " supported with --platform "
                        "syntax.\n");
}

formatted
-------------

void f2()
{
        fprintf(stderr, "Only one OpenCL device"
                " supported with --platform " "syntax.\n");
}

3. 'if' condition
-------------------

original
----------

// gpu_amd is a macro
void f3()
{
        if gpu_amd(this_is_a_macro) {
                do_something();
                do_something();
        }
}

formatted
-------------

void f3()
{
        if gpu_amd
        (this_is_a_macro) {
                do_something();
                do_something();
        }
}

For case 1, if we remove the '{}' which includes the `while`, the breakage
can be avoided.

void f1()
{
#if CON1
#if CON2
                while (condition) {
                        do_something();
                        do_something();
                }
#endif
#endif
}

For case 2, checkpatch.pl script can catch this problem. So we can
manually fix it.

For case 3, it can be avoided if we add '()' around the gpu_amd.

void f3()
{
        if (gpu_amd(this_is_a_macro)) {
                do_something();
                do_something();
        }
}

Generally, I think the indent and astyle help a lot, and it only causes
a few breakages. And we can use checkpatch.pl to check again.

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.