[Ardour-Dev] Hello, some tech talk, etc.

[Tim Mayberry]

On Thu, Jul 5, 2012 at 10:54 PM, Sakari Bergen
<sakari.bergen at beatwaves.net> wrote:
> On Thu, Jul 5, 2012 at 3:38 PM, Tim Mayberry <mojofunk at gmail.com> wrote:
>>
>> On Thu, Jul 5, 2012 at 1:37 PM, Paul Davis <paul at linuxaudiosystems.com>
>> wrote:
>> >
>> >
>> > On Wed, Jul 4, 2012 at 11:31 PM, Razvan Cojocaru <razvanco at gmx.net>
>> > wrote:
>> >>
>> >> >> 1) recursive mutexes are not acceptable. google around for
>> >> >> decades-old
>> >> >> criticisms and the story of how they came to be part of pthreads.
>> >> >
>> >> > +1 !!
>> >>
>> >> OK, so recursive mutexes are off-limits, but gotos are OK. :D
>> >
>> >
>> > goto's as a mean of avoiding multiple exit points from a function
>> > without
>> > creating excessively complex nested conditionals are definitely OK.
>> > gotos as
>> > a means of restarting a loop under odd conditions is not ideal since it
>> > can
>> > normally be done better without them, but sometimes  it just feels
>> > right.
>> >
>>
>> To me it seems like many of the uses of goto in the ardour source
>> result from a more "C style" of programming and with a little
>> refactoring many could be removed if that was a goal.
>>
>> I had a rough go at it this afternoon and removing about 20 out of the
>> first 25 or so I encountered in libardour was fairly straight forward:
>> https://github.com/mojofunk/ardour3-mingw/commits/goto
>>
>> about 30 less lines of code too.
>>
>> One other "C style" practice I notice a lot is using an int as a
>> return type to indicate success(0)/failure(-1) rather than
>> bool(success=true, failure=false). I find this a little confusing
>> sometimes when reading through the code being more used to C++ style.
>>
>> Tim.
>
>
>  I also think most of the gotos could be removed by using RAII more.
>
> I took a quick look at what you had done (just a couple of change sets), and
> noticed at least one error: you used scoped_ptr with new[], which will end
> up calling delete, instead of delete[] in the scoped_ptr's destructor. You
> should have used scoped_array instead.
>
> -Sakari-

Yes, I said it was rough :) I do understand the need for scoped_array.
Obviously any such changes would need testing and review if they were
to be applied.

Tim.