unused code in float8_to_char , formatting.c ?
Hi Guys,
Looking around the code Today, one of my helpful tools detected this dead code.
As far as I can see, it is actually unused call to strlen() in formatting.c, float8_to_char().
Diff attached.
Attachments:
formatting_dead_code.diffapplication/octet-stream; name=formatting_dead_code.diffDownload+0-1
On Thu, Apr 4, 2013 at 6:47 PM, Greg Jaskiewicz <gryzman@me.com> wrote:
Looking around the code Today, one of my helpful tools detected this dead code.
As far as I can see, it is actually unused call to strlen() in formatting.c, float8_to_char().
I poked at this a little and suggest the following somewhat more
extensive cleanup.
It seems to me that there are a bunch of these functions where len is
unconditionally initialized in NUM_TOCHAR_prepare and then used there.
Similarly in NUM_TOCHAR_cleanup. And then there's a chunk of each
individual function that does it a third time. Rather than use the
same variable in all three places, I've moved the variable
declarations to the innermost possible scope. Doing that revealed a
bunch of other, similar places where we can get rid of strlen() calls.
Does this version seem like a good idea?
...Robert
Attachments:
formatting_dead_code_v2.diffapplication/octet-stream; name=formatting_dead_code_v2.diffDownload+15-18
On 7 Apr 2013, at 05:14, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Apr 4, 2013 at 6:47 PM, Greg Jaskiewicz <gryzman@me.com> wrote:
Looking around the code Today, one of my helpful tools detected this dead code.
As far as I can see, it is actually unused call to strlen() in formatting.c, float8_to_char().I poked at this a little and suggest the following somewhat more
extensive cleanup.It seems to me that there are a bunch of these functions where len is
unconditionally initialized in NUM_TOCHAR_prepare and then used there.
Similarly in NUM_TOCHAR_cleanup. And then there's a chunk of each
individual function that does it a third time. Rather than use the
same variable in all three places, I've moved the variable
declarations to the innermost possible scope. Doing that revealed a
bunch of other, similar places where we can get rid of strlen() calls.Does this version seem like a good idea?
Looks more extensive :-)
On the quick glance, without a lot of testing it looks ok.
But the lack of test cases stressing all different cases in that file, makes it impossible to say that there's no regressions.
Personally I always feel uneasy making extensive changes in complicated code like this, without any integrated test case(s).
--
GJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Apr 7, 2013 at 12:14:29AM -0400, Robert Haas wrote:
On Thu, Apr 4, 2013 at 6:47 PM, Greg Jaskiewicz <gryzman@me.com> wrote:
Looking around the code Today, one of my helpful tools detected this dead code.
As far as I can see, it is actually unused call to strlen() in formatting.c, float8_to_char().I poked at this a little and suggest the following somewhat more
extensive cleanup.It seems to me that there are a bunch of these functions where len is
unconditionally initialized in NUM_TOCHAR_prepare and then used there.
Similarly in NUM_TOCHAR_cleanup. And then there's a chunk of each
individual function that does it a third time. Rather than use the
same variable in all three places, I've moved the variable
declarations to the innermost possible scope. Doing that revealed a
bunch of other, similar places where we can get rid of strlen() calls.Does this version seem like a good idea?
Robert, were you going to apply this patch from April?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Nov 30, 2013 at 9:01 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Sun, Apr 7, 2013 at 12:14:29AM -0400, Robert Haas wrote:
On Thu, Apr 4, 2013 at 6:47 PM, Greg Jaskiewicz <gryzman@me.com> wrote:
Looking around the code Today, one of my helpful tools detected this dead code.
As far as I can see, it is actually unused call to strlen() in formatting.c, float8_to_char().I poked at this a little and suggest the following somewhat more
extensive cleanup.It seems to me that there are a bunch of these functions where len is
unconditionally initialized in NUM_TOCHAR_prepare and then used there.
Similarly in NUM_TOCHAR_cleanup. And then there's a chunk of each
individual function that does it a third time. Rather than use the
same variable in all three places, I've moved the variable
declarations to the innermost possible scope. Doing that revealed a
bunch of other, similar places where we can get rid of strlen() calls.Does this version seem like a good idea?
Robert, were you going to apply this patch from April?
Done.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers