On 05/06/2012 10:14 PM, Simon Wunderlich wrote:
> Your patch does the trick, although I think this ugly function could use a rewrite.
> First counting bytes and then allocating this size to do exactly the same thing again
> is not really good style. If you would like to volunteer to do this job (or plan to
> work more on vis), please tell me, otherwise I'll put it in on my TODO list.

While I'am already at it, I guess I can also volunteer to do some more vis work :)

Besides cleanup, are there more ideas about the vis? A nice feature I can think about would be adding some freely chosen identification string (for example the hostname) to the vis data, this could make big graphs much more readable. I wonder though if this would be possible without breaking compatiblity.

I have some questions about the code though:

- Is there any reason vis_seq_print_text() allocates a buffer at all instead of just printing the data directy into the seq_file? Looking at the seq_printf implementation, there doesn't seem to be a problem calling it while holding the lock.
- In many places in the vis code hlist_for_each_entry_rcu() is used to iterate over the hash lists, even though all access to vis_hash is guarded by the vis_hash_lock, so it seems to be okay to just use hlist_for_each_entry(). In some functions, vis_seq_print_text() being one of them, rcu_read_lock/unlock pairs could be removed as well with this change. Do I overlook something?

I'll also drop by the batman IRC channel.

Matthias