Danny Backx wrote: > > Actually I did several modifications to try to follow the GNU coding > standards. That is what you're referring to, isn't it? > > Please tell me where exactly do I not follow one now ? I'll fix. >
Bummer, this really makes me sound a nitpicker. I would have preferred you hadn't committed the new patch without posting it to the list first. I had more comments. See here for reference: http://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting > * c-common.c (handle_exception_handler_attribute) : Add new handler to > support the __exception_handler__ attribute. Please move the attribute implementation into either arm.c, or better yet, reuse the SUBTARGET_ATTRIBUTE_TABLE mechanism already implemented for the "selectany" attribute. Since this is a target specific attribute it has no place in the common c code. > Modified: trunk/cegcc/src/gcc/gcc/c-common.c > +static tree > +handle_exception_handler_attribute (tree *node, tree name, > + tree args, > + int ARG_UNUSED (flags), > + bool *no_add_attrs) > +{ > + if (TREE_CODE (*node) == FUNCTION_DECL) > + { > + /* > + * We need to pass the name of the exception handler. The > + * right code then gets generated from config/arm/mingw32.h > + * or similar, the assembler and linker will do the hard work. > + * > + * FIX ME We don't support passing data to the exception handler. > + * > + * This should be possible though, by using an additional argument > + * which needs to fit in the dword (e.g. a pointer) and storing that > + * in the right field as we do with the exception handler. > + */ > + See the other comments in the file for correct formatting. Also, not applicable in this case, but Its two spaces after '.' to finish a sentence. And mind the indenting. /* This is comment 1. This is comment 2. This is comment 3 that flows into a new in a new line, and the end comment goes in the same line as the last. */ Also mind the tab/spacing. Tabs are preferred. 8 char wide tabs are mandatory. > + /* Handle mode attribute, handle_section_attribute, .. use args */ > + tree id = TREE_VALUE (args); > + tree attr = NULL_TREE; > + > +#if 1 > + attr = tree_cons (get_identifier ("exception_handler"), args, attr); > +#else > + /* Works too */ > + const char *x = IDENTIFIER_POINTER(id); > + > + attr = tree_cons (get_identifier ("exception_handler"), > + build_string (strlen (x), x), attr); > +#endif Please don't add dead code for no reason. If the first version is best, then remove the second. > +/* > + * called from ASM_DECLARE_FUNCTION_NAME in gcc/config/arm/wince-pe.h > + */ > +char * > +arm_exception_handler (FILE *fp, char *name, tree decl) > +{ > + tree attr, a2; Should be two spaces of indenting. Same throughout. Try running 'indent' on the file to see how it should be. Do it in a copy, since indent work in-place. And please only re-indent your part of the code. No use diverging from the main sources only on indenting. > +/* Give this another name so more sub-architectures can overrule ARM_DECLARE_FUNCTION_NAME > + * and still call ARM_PE_DECLARE_FUNCTION_NAME. The alternative is to duplicate the code > + * below to the sub-architectures, that's a bad idea. > + * This is currently done in wince-pe.h . */ Same comment about the formatting of the comment. Just look at the other comments in the file or other files. I find that the comment about duplicating code adds no value. \ > + { > \ > + if (arm_exception_handler(STREAM, NAME, DECL)) \ > + asm_fprintf (STREAM, ".L%s_end:\n", NAME); \ > + } > I prefer a do {} while (0), but that's just me. At this point, and since you will have to move code around, perhaps it would be better to revert the patch in full, and then post the new version here, and then apply in all at once again when it's OK. Cheers, Pedro Alves ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Cegcc-devel mailing list Cegcc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/cegcc-devel
