On 07/09/2018 02:33 PM, Dmitry Timoshkov wrote:
Jacek Caban jacek@codeweavers.com wrote:
On 07/09/2018 08:26 AM, Dmitry Timoshkov wrote:
@@ -173,9 +181,6 @@ static int tlb_read_byte(void) static void print_offset(void) { int i;
- printf("%04x: ", offset);
- for(i=0; i<indent; i++) printf(" ");
}
Why are you dropping printing offset?
It just adds noise. Once you compare old and new output perhaps you will agree with the change.
It's been a while since I worked with typelibs, but as far as I remember a lot of records use offsets and for that having them printed was useful. Would it make sense to have a command line option to control if they are printed?
For this and other helper function changes, it would be nice to have separated patches.
I considered splitting the whole patch into a series, but the dumper is a result of pretty old investigation for which I still have full commit sequence, but the end result is very different from the very first patch, and unless you would like to see the way of thinking there's really nothing interesting or fascinating there: once I've found a new .tlb for my collection I had to adapt the dumper. The changes in the helpers are part of this investiagtion work, and I don't see a nice way to split them in separate patches.
I'm probably fine with having SLTG dumper part as one huge patch. I didn't have deeper look at that part yet, unrelated changes caught my attention. Those are easy to split and not really related to core part of the patch. print_offset(), for example, could use a separated patch on its own (although I'm not sure if I like this one, as above). Adding ...h suffix to hex values is also unrelated to the core patch. Same for dump_binary(), it could be a separated patch.
Thanks, Jacek