Yes, you are right: function version is still 20Bytes bigger than macro version. Do you have a better solution to fix this problem?
> -----Original Message----- > From: David Sidrane <david.sidr...@nscdg.com> > Sent: Friday, July 31, 2020 6:29 PM > To: dev@nuttx.apache.org > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change > in pull request #1487: libc: Avoid ctype function to > evaluate the argument more than once > > > The normal function size is smaller than macro version. > > The comment and data seem to be in conflict. > > master:43255 is less than 'normal function':43275 > > Am I missing something? > > -----Original Message----- > From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com] > Sent: Friday, July 31, 2020 3:19 AM > To: dev@nuttx.apache.org > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change > in pull request #1487: libc: Avoid ctype function to > evaluate the argument more than once > > Here is the size report for stm32f103-minimum:nsh: > The master: > text data bss dec hex filename > 43255 108 2196 45559 b1f7 nuttx > With lookup table > patch(https://github.com/apache/incubator-nuttx/pull/1487): > text data bss dec hex filename > 43567 108 2196 45871 b32f nuttx > With normal function > patch(https://github.com/apache/incubator-nuttx/pull/1496): > text data bss dec hex filename > 43275 108 2196 45579 b20b nuttx > The normal function size is smaller than macro version. > > > -----Original Message----- > > From: Xiang Xiao <xiaoxiang781...@gmail.com> > > Sent: Friday, July 31, 2020 4:08 PM > > To: dev@nuttx.apache.org > > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on a > > change in pull request #1487: libc: Avoid ctype function to evaluate > > the argument more than once > > > > Let's restart the problem: it's definitely a bug that ctype macros > > evaluate it's argument more than once. If the code size is more > > important than the speed, let's change all macros to normal function > > that we get the correction behavior and save the code size at the same > > time. > > > > > -----Original Message----- > > > From: David Sidrane <david.sidr...@nscdg.com> > > > Sent: Friday, July 31, 2020 7:32 AM > > > To: dev@nuttx.apache.org > > > Subject: RE: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on > > > a change in pull request #1487: libc: Avoid ctype function to > > > evaluate the argument more than once > > > > > > > Is this something we should be concerned about? > > > > > > > > > Sorry if that came off wrong. I was agreeing with Greg's with the > > > former statement. Not the latter one. > > > > > > David > > > > > > -----Original Message----- > > > From: Brennan Ashton [mailto:bash...@brennanashton.com] > > > Sent: Thursday, July 30, 2020 12:57 PM > > > To: dev@nuttx.apache.org > > > Subject: Re: [GitHub] [incubator-nuttx] xiaoxiang781216 commented on > > > a change in pull request #1487: libc: Avoid ctype function to > > > evaluate the argument more than once > > > > > > On Thu, Jul 30, 2020, 12:40 PM David Sidrane > > > <david.sidr...@nscdg.com> > > > wrote: > > > > > > > -1 on more bloat > > > > > > > > > > That is not a constructive vote. No one thinks we should have more > > > bloat. > > > > > > The question is do we need to focus more on this. Some of the > > > changes are due to shortcuts we have taken in the past that are now > > > causing issues. The PR that spurred this is exactly this. The code > > > as it exists is wrong in that it has unexpected side effects which > > > is what Xiao > > is trying to address. > > > > > > I will acknowledge that there have been other changes where it was > > > more of a complexity trade-off. Even on those there was discussion > > > about the impact. > > > > > > > > > > It is a simple matter to see the cost of a PR if we add bloaty > > > > (https://github.com/google/bloaty) to ci. > > > > > > > > > > Great please file the simple PR to add it. This is not trivial as we > > > need to manage the artifacts from previous builds to do the > > > comparison or double the build time. We heard you last time, but > > someone has to do to the actual work. > > > > > > --Brennan
