On 20.10.25 15:31, Chao Li wrote:
On Oct 20, 2025, at 17:50, Peter Eisentraut <[email protected]> wrote:
The file formatting.c contains some hard to read and understand code. For the
attached patch series, I made a few more or less mechanical passes over it to
modernize the code a bit, renamed some symbols to be clearer, adjusted some
types to make things more self-documenting, removed some redundant code to make
some things more compact. I hope this helps a bit.
I have committed this patch set. Thanks for checking.
0004 - Changes “char *” to “const char *” wherever possible.
I found some “text *” can be “const text *”:
I added those.
0007 - I am not convinced with this change. Combining two constant string into
one causes the line too long, exceeding 80 columns.
We have been moving toward not breaking up string literals, except where
they contain a line break. This makes it easier to grep for error
messages, for example.
0010 - Changes TmFromChar.clock from int to bool.
A suggestion is that maybe we can move the new field “clock_12_hour” next to
“bool has_tz”, so that size of the structure is reduced by 4 bytes.
I don't think we need to worry about structure size here. (If we did,
we could change many fields to short int, probably.) The current order
seems pretty logical. So I kept it.
0013 - Changes fill_str() to return void.
```
-static char *
+static void
fill_str(char *str, int c, size_t maxlen)
{
memset(str, c, maxlen);
str[maxlen] = '\0';
- return str;
}
```
This function has no comment, so I am not sure what “maxlen” exactly mean.
Usually, we do “str[maxlen-1] = ‘\0’ because maxlen usually implies max length
of the buffer. But this function seems to fill maxlen of c into str, then
“maxlen” might not be a good name, “count” could be better.
Yes, this is a bit confusing. I added a function comment to explain this.
