2009/2/23 Dominik Vogt <[email protected]>: > On Mon, Feb 23, 2009 at 09:21:03PM +0000, Thomas Adam wrote: >> 2009/2/23 Dominik Vogt <[email protected]>: >> - I can't say I like the magic numbers in the malloc line: > > :-P > > I couldn't think of anything better, and I think the previous > allocation was too small.
Perhaps -- I did toy with looking at the actual size of the charmap_t[] array at each instance it was being called (once as per key_bindings, the other as win_contexts) -- but no worries, expect a potential comment on this nevertheless, since if this ever comes back to haunt myself and you, I don't want to be left scratching my head as to why it's * 8 +1. :) >> allmods = safemalloc(sizeof(table->value) * 8 + 1); >> >> However, I know why they're there -- I might add a comment there if I >> really feel strongly about it. :) >> >> This approach is better in that it reduces memory allocation a little. >> >> I don't have a problem with it, and it seems to work well. > >> Your changes to fvwm/bindings.c:print_bindings() aren't as extensive. >> I like (as trivial as it sounds) the block around the code after the >> case statement -- that does make sense. > > I like to use that to limit the lifetime of stack variables. And > it's often a good way to document blocks of code. Oh absolutely -- it's a technique I use often and embarrasingly neglected to use here. Thanks though for catching it. >> The only query I have in print_bindings() is I am well aware I have >> done nothing with "b->Action2", and whilst it is set in places, I am a >> bit unsure when and if it is actually ever used -- at least, it hasn't >> had any noticeable effects when dumping out bindings. Perhaps though >> it's worth considering -- and I'd be happy to do so if I understood >> its use. > > It's only used in FvwmIconMan. Thanks again -- that was my understanding as well, but it is always good to be sure. -- Thomas Adam
