1. Yes, you are right, that is another way to fix the warnings. I did not fix it that way because I thought that mixing allocated data and constant data in the same array was bad style, worse than leaking a little more memory in a generator program.
2. I did not think C/C++ would allocate stack space for const data. In any case, I will revert that part of the change as it is not necessary to fix the warnings, which was my main goal. On Mon, Oct 11, 2010 at 12:49 PM, Jian-Xin Lai <laij...@gmail.com> wrote: > Hi David, > > Please refer my comments in below. > > 2010/10/11 David Coakley <dcoak...@gmail.com> >> >> Hi Jian-Xin, >> >> Thanks for looking at the changes. >> >> 1. Since sometimes the names are constructed using sprintf, I don't >> see how to make comp_name const. We could allocate fixed-length >> buffers if we want to avoid the strdup() and the leak with something >> like: >> >> static char comp_name[MAX_LISTING_OPERANDS][MAX_COMP_NAME_LEN]; > > Convertion from char * (return type of strdup()) to const char * is safe. > Since we don't release the memory allocated by strdup, it looks your change > increases the memory leakage. The comp_name can be defined like: > static const char *comp_name[MAX_LISTING_OPERANDS]; > >> >> 2. I thought the old declaration was confusing by being unusual: >> >> const char * const isa_exec_type_format = "..."; >> >> My change is to use the common C idiom for string constants: >> >> const char isa_exec_type_format[] = "..."; >> >> This form is preferred because it avoids introducing an unnecessary >> pointer. > > Since isa_exec_type_format is read only, we don't need to allocate space on > stack to hold the string. The second 'const' in original code ensures the > value of the pointer itself to be const. > >> >> -David Coakley / AMD Open Source Compiler Engineering >> >> On Mon, Oct 11, 2010 at 11:22 AM, Jian-Xin Lai <laij...@gmail.com> wrote: >> > Some comments: >> > 1. In osprey/common/targ_info/generate/isa_print_gen.cxx(line 188) and >> > osprey/common/targ_info/generate/isa_pack_gen.cxx (line 243): >> > I wonder it's better to change the type of comp_name from char* to const >> > char* so that we don't need to call strdup on string literal. >> > >> > 2. In osprey/common/targ_info/generate/isa_bundle_gen.cxx (line 349) >> > It looks like changing the type of isa_exec_type_format from pointer to >> > array is unnecessary. >> > >> > 2010/10/11 David Coakley <dcoak...@gmail.com> >> >> >> >> Hi Open64 gatekeepers, >> >> >> >> Has someone had a chance to review the attached cleanup patch? >> >> >> >> On Fri, Oct 1, 2010 at 4:26 PM, David Coakley <dcoak...@gmail.com> >> >> wrote: >> >> > Hi all, >> >> > >> >> > I am submitting a proposal to eliminate the scheduling info DSOs and >> >> > instead incorporate that info directly into the backend. The primary >> >> > motivation is portability. The proposal is described in more detail >> >> > in the attachment "no_si_dso.txt". >> >> > >> >> > Before making these changes, I thought it was a good idea to clean up >> >> > the compilation warnings in targ_info and its generated code, and to >> >> > fix some known portability problems (bugs 622, 624, 658). The patch >> >> > "ti_cleanup.diff" is a code cleanup patch that accomplishes those >> >> > goals without making any changes in the current behavior. The >> >> > details >> >> > are in the tentative log message, "log_msg.txt". >> >> > >> >> > Could a gatekeeper please review the patch? >> >> > >> >> > I have prototyped the proposed targ_info DSO changes for x8864. I >> >> > would like feedback from some of the gatekeepers and especially the >> >> > maintainers for other targets -- I will need your help since the >> >> > changes affect all targets. Thanks, >> >> > >> >> > -David Coakley / AMD Open Source Compiler Engineering >> >> > >> >> >> >> >> >> >> >> ------------------------------------------------------------------------------ >> >> Beautiful is writing same markup. Internet Explorer 9 supports >> >> standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3. >> >> Spend less time writing and rewriting code and more time creating >> >> great >> >> experiences on the web. Be a part of the beta today. >> >> http://p.sf.net/sfu/beautyoftheweb >> >> _______________________________________________ >> >> Open64-devel mailing list >> >> Open64-devel@lists.sourceforge.net >> >> https://lists.sourceforge.net/lists/listinfo/open64-devel >> >> >> > >> > >> > >> > -- >> > Regards, >> > Lai Jian-Xin >> > > > > > -- > Regards, > Lai Jian-Xin > ------------------------------------------------------------------------------ Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
