Re: unused code in float8_to_char , formatting.c ?

From: Greg Jaskiewicz <gryzman(at)me(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Greg Jaskiewicz <gryzman(at)me(dot)com>, "pgsql-hackers(at)postgresql(dot)org Hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: unused code in float8_to_char , formatting.c ?
Date: 2013-04-07 09:03:48
Message-ID: BFF1A6B0-A438-4612-AE63-BEA5BABA8D9C@me.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 7 Apr 2013, at 05:14, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Apr 4, 2013 at 6:47 PM, Greg Jaskiewicz <gryzman(at)me(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2013-04-07 11:54:28 Re: doc hdparm also support SATA
Previous Message Jaime Casanova 2013-04-07 07:07:20 Re: corrupt pages detected by enabling checksums