From: Daniel H. <dan...@li...> - 2008-04-07 15:43:32
Attachments:
padding_fix.patch
|
Hi, the calculation of padding byte amount was wrong in parsing functionality. Please review and give comments. Thanks in advance. Kind regards, Daniel |
From: John L. <le...@mo...> - 2008-04-07 16:25:26
|
On Mon, Apr 07, 2008 at 05:42:20PM +0200, Daniel Hansel wrote: > - padding_count = (((rec->total_size + 7) & 7) ^ 7); > + /* padding bytes are calculated over the complete record > + * (i.e. header + symbol name + code) > + */ > + padding_count = (((sizeof(*rec) + strlen(entry->symbol_name) + 1 + entry->code_size + 7) & 7) ^ 7); Please use an intermediary variable for 'sizeof...code_size' instead of this super-long line. Also, maybe time for the & 7 stuff to become a macro wit a sensible name? cheers john |
From: Daniel H. <dan...@li...> - 2008-04-08 00:11:30
Attachments:
padding_fix.patch
|
Hi John, you're right. I have seen that the new "padding" macro would also be useful for creating dump file functionality. I hope that the new patch is good to be integrated. Please review and give comments. Thanks in advance. Kind regards, Daniel John Levon wrote: > On Mon, Apr 07, 2008 at 05:42:20PM +0200, Daniel Hansel wrote: > >> - padding_count = (((rec->total_size + 7) & 7) ^ 7); >> + /* padding bytes are calculated over the complete record >> + * (i.e. header + symbol name + code) >> + */ >> + padding_count = (((sizeof(*rec) + strlen(entry->symbol_name) + 1 + entry->code_size + 7) & 7) ^ 7); > > Please use an intermediary variable for 'sizeof...code_size' instead of > this super-long line. Also, maybe time for the & 7 stuff to become a > macro wit a sensible name? > > cheers > john |
From: John L. <le...@mo...> - 2008-04-08 00:42:17
|
On Tue, Apr 08, 2008 at 02:11:16AM +0200, Daniel Hansel wrote: > + * Macro to calculate count of padding bytes > + * to extend a size to be 8-byte aligned. */ > +#define PADDING_8ALIGNED(x) (((x + 7) & 7) ^ 7) This needs to be: #define PADDING_8ALIGNED(x) ((((x) + 7) & 7) ^ 7) Looks fine otherwise cheers john |