On Mon, Oct 8, 2012 at 6:10 PM, Jeff Darcy <jdarcy@xxxxxxxxxx> wrote:
KP/Jeff,
It was me who introduced inner functions with patch http://review.gluster.org/3829.
The reason was to keep the changes as minimal as possible within that particular patch. I have no objections to remove the inner functions completely from the codebase.
But it was good exercise to understand the reason for crash though :-)
Regards,
Amar
On October 8, 2012 7:02:34 AM Krishnan Parthasarathi <kparthas@xxxxxxxxxx> wrote:Ugh. I noticed this pattern while I was looking at some AFR stuff recently. I thought it was rather clever, and pondered a bit about how it might be implemented in gcc/libc. Apparently it was a bit too clever, and the implementation leaves something to be desired. An inner function might be more elegant than defining private structures to pass through our own context pointer, but in the interest of both portability and not having to debug compiler code I think changing these to work the "old fashioned" way would actually be a good thing.
I have been rewriting some of the volume operations (like volume-start,
volume-add-brick, volume-remove-brick) using synctask library (aka syncops).
This change has the following immediate benefits,
- volume-start would return success/failure depending on the success/failure of
brick process(es) spawned.
- would make glusterd's epoll thread 'more' available.
</context>
While I was making the changes in http://review.gluster.com/3969, I noticed that
whenever the code executing on a synctask called into dict_foreach, which was supplied
a function ptr, defined as an inner function, glusterd crashed. When I rewrote
inner function as a static function, glusterd wouldn't crash.
Has anyone seen or can explain (or give possible leads to analyse) this behaviour?
FWIW, inner functions are only available as part of GNU extensions to C. So, I
assumed it is not such a bad thing to move the inner functions 'out', in my patch.
KP/Jeff,
It was me who introduced inner functions with patch http://review.gluster.org/3829.
The reason was to keep the changes as minimal as possible within that particular patch. I have no objections to remove the inner functions completely from the codebase.
But it was good exercise to understand the reason for crash though :-)
Regards,
Amar
