> On Dec 29, 2017, at 8:55 AM, Geert Janssens <geert.gnuc...@kobaltwit.be>
> wrote:
>
> Op vrijdag 29 december 2017 17:22:29 CET schreef John Ralls:
>>> On Dec 29, 2017, at 7:41 AM, Geert Janssens <geert.gnuc...@kobaltwit.be>
>>> wrote:>
>>> Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
>>>> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
>>>> Author: John Ralls <jra...@ceridwen.us>
>>>> Date: Tue Dec 26 13:01:50 2017 -0800
>>>>
>>>> Enforce -Werror on C++ files and fix resulting errors.
>>>
>>> <snip>
>>>
>>>> diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
>>>> b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index
>>>> 9402f38..1bfa08e 100644
>>>> --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
>>>> +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
>>>> @@ -355,12 +355,12 @@ static void csv_tximp_preview_currency_fmt_sel_cb
>>>> (GtkComboBox* format_selector, info->preview_update_currency_format();
>>>> }
>>>>
>>>> -void csv_tximp_preview_col_type_changed_cb (GtkComboBox* cbox,
>>>> CsvImpTransAssist* info)
>>>> +static void csv_tximp_preview_col_type_changed_cb
>>>> (GtkComboBox* cbox, CsvImpTransAssist* info) {
>>>>
>>>> info->preview_update_col_type (cbox);
>>>>
>>>> }
>>>
>>> Apparently your new compile options require function declarations or to
>>> declare local functions static. What was wrong with the previous behavior
>>> that you choose to enforce this ?
>>
>> CMakeLists.txt didn’t set any warning or error flags on C++ files, not even
>> -Werror, so I just copied over the C flags and then adjusted them to deal
>> with g++ wanting -Wmissing-declarations instead of -Wmissing-prototypes.
>>
>> I just now looked at configure.ac and the flags there are rather different,
>> with neither missing-declarations nor missing-prototypes on either C or
>> C++.
>>
>> configure.ac:
>> AM_CFLAGS=-Wall -Werror -Wno-unused -Wno-deprecated-declarations
>> -Wmissing-prototypes -Wmissing-prototypes
>> -Werror-implicit-function-declaration
>>
>> AM_CXXFLAGS=-Wall -Werror -Wno-unused
>>
>> CMakeLists.txt:
>> CMAKE_C_FLAGS=-Werror -Wdeclaration-after-statement -Wno-pointer-sign -Wall
>> -Wunused -Wmissing-prototypes -Wmissing-declarations -Wno-unused
>> -Wno-deprecated-declarations -std=gnu11 CMAKE_CXX_FLAGS=-std=gnu++11
>> -Werror -Wall -Wmissing-declarations -Wno-unused
>>
>> I think that we should remove -Wno-unused from CXXFLAGS in general and only
>> use it where needed on external sources. GnuCash C++ code is all new and it
>> should all compile clean. For that code the more warnings the better: It
>> gets the compiler to help us write better code.
>>
> Ok, I fully agree.
>
>> I’m in favor of having missing-prototypes/missing-declarations and declaring
>> compilation-unit-local functions static to make their scope explicit.
>>
> Ok, that's a good motivation. Just to be clear for me, having a function
> static is different from having a static variable, right ? As I understand it
> the latter is bad for thread safety, the former only ensures its local scope.
Yes, and static in a class declaration means something else again.
Careless use of shared state will cause thread safety issues, yes. But static
variables aren’t the main source of the shared state problem: Promiscuous
passing of pointers to heap/free store variables is. Consider the possibility
that two threads get the pointer to a particular split from QofContainer and
proceed to edit it. There’s no mutex in QofInstance’s begin_edit/commit_edit,
so the results are almost guaranteed to be interesting.
Regards,
John Ralls
_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
