Horton

Thread

Commits

  1. pg_waldump: Allow hexadecimal values for -t/--timeline option

  2. doc: Additional information about timeline ID hexadecimal format

  1. Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-01-27T13:52:19Z

    Hi,

I've been puzzled by this message:

~~~
LOG:  fetching timeline history file for timeline 17 from primary server
FATAL:  could not receive timeline history file from the primary server: 
ERROR:  could not open file "pg_xlog/00000011.history": No such file or 
directory
~~~

It took me a while to understand that the timeline id 11 in hexadecimal 
is the same as the timeline id 17 in decimal.

It appears that the first message is formatted with %u instead of %X, 
and there some others places with the some format, while WAL filename 
and history file used hexadecimal.

There is another place where timeline id is used : pg_waldump, and in 
these tools, timeline id ( -t or --timeline ) should be given in 
decimal, while filename gives it in hexadecimal : imho, it's not 
user-friendly, and can lead to user's bad input for timeline id.

The attached patch proposes to change the format of timelineid from %u 
to %X.

Regarding .po files, I don't know how to manage them. Is there any 
routine to spread the modifications? Or should I identify and change 
each message?


best regards,

-- 
Sébastien

  2. Re: Timeline ID hexadecimal format

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-01-27T14:55:11Z

    On 27.01.23 14:52, Sébastien Lardière wrote:
> The attached patch proposes to change the format of timelineid from %u 
> to %X.

I think your complaint has merit.  But note that if we did a change like 
this, then log files or reports from different versions would have 
different meaning without a visual difference, which is kind of what you 
complained about in the first place.  At least we should do something 
like 0x%X.

> Regarding .po files, I don't know how to manage them. Is there any 
> routine to spread the modifications? Or should I identify and change 
> each message?

Don't worry about this.  This is handled elsewhere.

  3. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-01-27T16:17:35Z

    On 27/01/2023 15:55, Peter Eisentraut wrote:
> On 27.01.23 14:52, Sébastien Lardière wrote:
>> The attached patch proposes to change the format of timelineid from 
>> %u to %X.
>
> I think your complaint has merit.  But note that if we did a change 
> like this, then log files or reports from different versions would 
> have different meaning without a visual difference, which is kind of 
> what you complained about in the first place.  At least we should do 
> something like 0x%X.

Indeed, but the messages that puzzled was in one log file, just 
together, not in some differents versions.

But yes, it should be documented somewhere, actually, I can't find any 
good place for that,

While digging, It seems that recovery_target_timeline should be given in 
decimal, not in hexadecimal, which seems odd to me ; and pg_controldata 
use decimal too, not hexadecimal…

So, if this idea is correct, the given patch is not enough.

Anyway, do you think it is a good idea or not ?


>
>> Regarding .po files, I don't know how to manage them. Is there any 
>> routine to spread the modifications? Or should I identify and change 
>> each message?
>
> Don't worry about this.  This is handled elsewhere.
>

nice,


regards,


-- 
Sébastien

  4. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-01-30T16:05:36Z

    On 27/01/2023 15:55, Peter Eisentraut wrote:
> On 27.01.23 14:52, Sébastien Lardière wrote:
>> The attached patch proposes to change the format of timelineid from 
>> %u to %X.
>
> I think your complaint has merit.  But note that if we did a change 
> like this, then log files or reports from different versions would 
> have different meaning without a visual difference, which is kind of 
> what you complained about in the first place.  At least we should do 
> something like 0x%X.
>
Hi,

Here's the patch with the suggested format ; plus, I add some note in 
the documentation about recovery_target_timeline, because I don't get 
how strtoul(), with the special 0 base parameter can work without 0x 
prefix ; I suppose that nobody use it.

I also change pg_controldata and the usage of this output by pg_upgrade. 
I let internal usages unchanded : content of backup manifest and content 
of history file.

Should I open a commitfest entry, or is it too soon ?

regards,

-- 
Sébastien

  5. Re: Timeline ID hexadecimal format

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-01-31T09:53:21Z

    On 30.01.23 17:05, Sébastien Lardière wrote:
> 
> Here's the patch with the suggested format ; plus, I add some note in 
> the documentation about recovery_target_timeline, because I don't get 
> how strtoul(), with the special 0 base parameter can work without 0x 
> prefix ; I suppose that nobody use it.
> 
> I also change pg_controldata and the usage of this output by pg_upgrade. 
> I let internal usages unchanded : content of backup manifest and content 
> of history file.
> 
> Should I open a commitfest entry, or is it too soon ?

It is not too soon.  (The next commitfest is open for new patch 
submissions as soon as the current one is "in progress", which closes it 
for new patches.)

  6. Re: Timeline ID hexadecimal format

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2023-01-31T11:26:39Z

    On Mon, Jan 30, 2023 at 9:35 PM Sébastien Lardière
<sebastien@lardiere.net> wrote:
>
> On 27/01/2023 15:55, Peter Eisentraut wrote:
> > On 27.01.23 14:52, Sébastien Lardière wrote:
> >> The attached patch proposes to change the format of timelineid from
> >> %u to %X.
> >
> > I think your complaint has merit.  But note that if we did a change
> > like this, then log files or reports from different versions would
> > have different meaning without a visual difference, which is kind of
> > what you complained about in the first place.  At least we should do
> > something like 0x%X.
> >
> Hi,
>
> Here's the patch with the suggested format ; plus, I add some note in
> the documentation about recovery_target_timeline, because I don't get
> how strtoul(), with the special 0 base parameter can work without 0x
> prefix ; I suppose that nobody use it.
>
> I also change pg_controldata and the usage of this output by pg_upgrade.
> I let internal usages unchanded : content of backup manifest and content
> of history file.

The patch seems to have some special/unprintable characters in it. I
see a lot ^[[ in there. I can't read the patch because of that.

-- 
Best Wishes,
Ashutosh Bapat

  7. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-01-31T12:52:57Z

    On 31/01/2023 12:26, Ashutosh Bapat wrote:
> On Mon, Jan 30, 2023 at 9:35 PM Sébastien Lardière
> <sebastien@lardiere.net> wrote:
>> On 27/01/2023 15:55, Peter Eisentraut wrote:
>>> On 27.01.23 14:52, Sébastien Lardière wrote:
>>>> The attached patch proposes to change the format of timelineid from
>>>> %u to %X.
>>> I think your complaint has merit.  But note that if we did a change
>>> like this, then log files or reports from different versions would
>>> have different meaning without a visual difference, which is kind of
>>> what you complained about in the first place.  At least we should do
>>> something like 0x%X.
>>>
>> Hi,
>>
>> Here's the patch with the suggested format ; plus, I add some note in
>> the documentation about recovery_target_timeline, because I don't get
>> how strtoul(), with the special 0 base parameter can work without 0x
>> prefix ; I suppose that nobody use it.
>>
>> I also change pg_controldata and the usage of this output by pg_upgrade.
>> I let internal usages unchanded : content of backup manifest and content
>> of history file.
> The patch seems to have some special/unprintable characters in it. I
> see a lot ^[[ in there. I can't read the patch because of that.
>
Sorry for that, it was the --color from git diff, it's fixed, I hope, 
thank you

regards,

-- 
Sébastien

  8. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-01-31T13:03:29Z

    On 31/01/2023 10:53, Peter Eisentraut wrote:
> On 30.01.23 17:05, Sébastien Lardière wrote:
>>
>> Here's the patch with the suggested format ; plus, I add some note in 
>> the documentation about recovery_target_timeline, because I don't get 
>> how strtoul(), with the special 0 base parameter can work without 0x 
>> prefix ; I suppose that nobody use it.
>>
>> I also change pg_controldata and the usage of this output by 
>> pg_upgrade. I let internal usages unchanded : content of backup 
>> manifest and content of history file.
>>
>> Should I open a commitfest entry, or is it too soon ?
>
> It is not too soon.  (The next commitfest is open for new patch 
> submissions as soon as the current one is "in progress", which closes 
> it for new patches.)


Done : https://commitfest.postgresql.org/42/4155/


-- 
Sébastien

  9. Re: Timeline ID hexadecimal format

    Greg Stark <stark@mit.edu> — 2023-01-31T19:16:06Z

    I actually find it kind of annoying that we use hex strings for a lot
of things where they don't add any value. Namely Transaction ID and
LSNs. As a result it's always a bit of a pain to ingest these in other
tools or do arithmetic on them. Neither is referring to memory or
anything where powers of 2 are significant so it really doesn't buy
anything in making them easier to interpret either.

I don't see any advantage in converting every place where we refer to
timelines into hex and then having to refer to things like timeline
1A. It doesn't seem any more intuitive to someone understanding what's
going on than referring to timeline 26.

The fact that the *filename* has it encoded in hex is an
implementation detail and really gets exposed here because it's giving
you the underlying system error that caused the problem. The confusion
only arises when the two are juxtaposed. A hint or something just in
that case might be enough?

  10. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-02-01T16:54:43Z

    On 31/01/2023 20:16, Greg Stark wrote:
> The fact that the *filename* has it encoded in hex is an
> implementation detail and really gets exposed here because it's giving
> you the underlying system error that caused the problem.


It's an implementation detail, but an exposed detail, so, people refer 
to the filename to find the timeline ID (That's why it happened to me)


>   The confusion
> only arises when the two are juxtaposed. A hint or something just in
> that case might be enough?
>
>

Thanks, i got your point.

  Note that my proposal was to remove the ambiguous notation which 
happen in some case (as in 11 <-> 17). A hint is useless in most of the 
case, because there is no ambiguous. That's why i though format 
hexadecimal everywhere.


At least, can I propose to improve the documentation to expose the fact 
that the timeline ID is exposed in hexadecimal in filenames but must be 
used in decimal in recovery_target_timeline and pg_waldump ?


regards,

-- 
Sébastien

  11. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-02-24T16:27:21Z

    On 31/01/2023 20:16, Greg Stark wrote:
> A hint or something just in
> that case might be enough?

It seems to be a -1 ;

let's try to improve the documentation, with the attached patch

best regards,

-- 
Sébastien

  12. Re: Timeline ID hexadecimal format

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-02T08:12:41Z

    On 24.02.23 17:27, Sébastien Lardière wrote:
> diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
> index be05a33205..7e26b51031 100644
> --- a/doc/src/sgml/backup.sgml
> +++ b/doc/src/sgml/backup.sgml
> @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p'
>       you like, add comments to a history file to record your own notes about
>       how and why this particular timeline was created.  Such comments will be
>       especially valuable when you have a thicket of different timelines as
> -    a result of experimentation.
> +    a result of experimentation. In both WAL segment file names and history files,
> +    the timeline ID number is expressed in hexadecimal.
>      </para>
>   
>      <para>

I think here it would be more helpful to show actual examples.  Like, 
here is a possible file name, this is what the different parts mean.

> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
> index e5c41cc6c6..3b5d041d92 100644
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml
> @@ -4110,7 +4110,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
>           current when the base backup was taken.  The
>           value <literal>latest</literal> recovers
>           to the latest timeline found in the archive, which is useful in
> -        a standby server.  <literal>latest</literal> is the default.
> +        a standby server. A numerical value expressed in hexadecimal must be
> +        prefixed with <literal>0x</literal>, for example <literal>0x11</literal>.
> +        <literal>latest</literal> is the default.
>          </para>
>   
>          <para>

This applies to all configuration parameters, so it doesn't need to be 
mentioned explicitly for individual ones.

> diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
> index 343f0482a9..4ae8f2ebdd 100644
> --- a/doc/src/sgml/ref/pg_waldump.sgml
> +++ b/doc/src/sgml/ref/pg_waldump.sgml
> @@ -215,7 +215,8 @@ PostgreSQL documentation
>          <para>
>           Timeline from which to read WAL records. The default is to use the
>           value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the
> -        default is 1.
> +        default is 1. The value must be expressed in decimal, contrary to the hexadecimal
> +        value given in WAL segment file names and history files.
>          </para>
>         </listitem>
>        </varlistentry>

Maybe this could be fixed instead?

  13. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-03-03T15:52:01Z

    On 02/03/2023 09:12, Peter Eisentraut wrote:
> On 24.02.23 17:27, Sébastien Lardière wrote:
>> diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
>> index be05a33205..7e26b51031 100644
>> --- a/doc/src/sgml/backup.sgml
>> +++ b/doc/src/sgml/backup.sgml
>> @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p'
>>       you like, add comments to a history file to record your own 
>> notes about
>>       how and why this particular timeline was created.  Such 
>> comments will be
>>       especially valuable when you have a thicket of different 
>> timelines as
>> -    a result of experimentation.
>> +    a result of experimentation. In both WAL segment file names and 
>> history files,
>> +    the timeline ID number is expressed in hexadecimal.
>>      </para>
>>        <para>
>
> I think here it would be more helpful to show actual examples. Like, 
> here is a possible file name, this is what the different parts mean.

So you mean explain the WAL filename and the history filename ? Is it 
the good place for it ?


>
>> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
>> index e5c41cc6c6..3b5d041d92 100644
>> --- a/doc/src/sgml/config.sgml
>> +++ b/doc/src/sgml/config.sgml
>> @@ -4110,7 +4110,9 @@ restore_command = 'copy 
>> "C:\\server\\archivedir\\%f" "%p"'  # Windows
>>           current when the base backup was taken.  The
>>           value <literal>latest</literal> recovers
>>           to the latest timeline found in the archive, which is 
>> useful in
>> -        a standby server. <literal>latest</literal> is the default.
>> +        a standby server. A numerical value expressed in hexadecimal 
>> must be
>> +        prefixed with <literal>0x</literal>, for example 
>> <literal>0x11</literal>.
>> +        <literal>latest</literal> is the default.
>>          </para>
>>            <para>
>
> This applies to all configuration parameters, so it doesn't need to be 
> mentioned explicitly for individual ones.

Probably, but is there another parameter with the same consequence ?

worth it to document this point globally ?


>
>> diff --git a/doc/src/sgml/ref/pg_waldump.sgml 
>> b/doc/src/sgml/ref/pg_waldump.sgml
>> index 343f0482a9..4ae8f2ebdd 100644
>> --- a/doc/src/sgml/ref/pg_waldump.sgml
>> +++ b/doc/src/sgml/ref/pg_waldump.sgml
>> @@ -215,7 +215,8 @@ PostgreSQL documentation
>>          <para>
>>           Timeline from which to read WAL records. The default is to 
>> use the
>>           value in <replaceable>startseg</replaceable>, if that is 
>> specified; otherwise, the
>> -        default is 1.
>> +        default is 1. The value must be expressed in decimal, 
>> contrary to the hexadecimal
>> +        value given in WAL segment file names and history files.
>>          </para>
>>         </listitem>
>>        </varlistentry>
>
> Maybe this could be fixed instead?
>
>

Indeed, and strtoul is probably a better option than sscanf, don't you 
think ?



-- 
Sébastien

  14. Re: Timeline ID hexadecimal format

    Robert Haas <robertmhaas@gmail.com> — 2023-03-03T16:04:14Z

    On Tue, Jan 31, 2023 at 2:16 PM Greg Stark <stark@mit.edu> wrote:
> I don't see any advantage in converting every place where we refer to
> timelines into hex and then having to refer to things like timeline
> 1A. It doesn't seem any more intuitive to someone understanding what's
> going on than referring to timeline 26.

The point, though, is that the WAL files we have on disk already say
1A. If we change the log messages to match, that's easier for users.
We could alternatively change the naming convention for WAL files on
disk, but that feels like a much bigger compatibility break.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  15. Re: Timeline ID hexadecimal format

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-06T17:04:08Z

    On 03.03.23 16:52, Sébastien Lardière wrote:
> On 02/03/2023 09:12, Peter Eisentraut wrote:
>> On 24.02.23 17:27, Sébastien Lardière wrote:
>>> diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
>>> index be05a33205..7e26b51031 100644
>>> --- a/doc/src/sgml/backup.sgml
>>> +++ b/doc/src/sgml/backup.sgml
>>> @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p'
>>>       you like, add comments to a history file to record your own 
>>> notes about
>>>       how and why this particular timeline was created.  Such 
>>> comments will be
>>>       especially valuable when you have a thicket of different 
>>> timelines as
>>> -    a result of experimentation.
>>> +    a result of experimentation. In both WAL segment file names and 
>>> history files,
>>> +    the timeline ID number is expressed in hexadecimal.
>>>      </para>
>>>        <para>
>>
>> I think here it would be more helpful to show actual examples. Like, 
>> here is a possible file name, this is what the different parts mean.
> 
> So you mean explain the WAL filename and the history filename ? Is it 
> the good place for it ?

Well, your patch says, by the way, the timeline ID in the file is 
hexadecimal.  Then one might ask, what file, what is a timeline, what 
are the other numbers in the file, etc.  It seems very specific in this 
context.  I don't know if the format of these file names is actually 
documented somewhere.

>>> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
>>> index e5c41cc6c6..3b5d041d92 100644
>>> --- a/doc/src/sgml/config.sgml
>>> +++ b/doc/src/sgml/config.sgml
>>> @@ -4110,7 +4110,9 @@ restore_command = 'copy 
>>> "C:\\server\\archivedir\\%f" "%p"'  # Windows
>>>           current when the base backup was taken.  The
>>>           value <literal>latest</literal> recovers
>>>           to the latest timeline found in the archive, which is 
>>> useful in
>>> -        a standby server. <literal>latest</literal> is the default.
>>> +        a standby server. A numerical value expressed in hexadecimal 
>>> must be
>>> +        prefixed with <literal>0x</literal>, for example 
>>> <literal>0x11</literal>.
>>> +        <literal>latest</literal> is the default.
>>>          </para>
>>>            <para>
>>
>> This applies to all configuration parameters, so it doesn't need to be 
>> mentioned explicitly for individual ones.
> 
> Probably, but is there another parameter with the same consequence ?
> 
> worth it to document this point globally ?

It's ok to mention it again.  We do something similar for example at 
unix_socket_permissions.  But maybe with more context, like "If you want 
to specify a timeline ID hexadecimal (for example, if extracted from a 
WAL file name), then prefix it with a 0x".

>>> diff --git a/doc/src/sgml/ref/pg_waldump.sgml 
>>> b/doc/src/sgml/ref/pg_waldump.sgml
>>> index 343f0482a9..4ae8f2ebdd 100644
>>> --- a/doc/src/sgml/ref/pg_waldump.sgml
>>> +++ b/doc/src/sgml/ref/pg_waldump.sgml
>>> @@ -215,7 +215,8 @@ PostgreSQL documentation
>>>          <para>
>>>           Timeline from which to read WAL records. The default is to 
>>> use the
>>>           value in <replaceable>startseg</replaceable>, if that is 
>>> specified; otherwise, the
>>> -        default is 1.
>>> +        default is 1. The value must be expressed in decimal, 
>>> contrary to the hexadecimal
>>> +        value given in WAL segment file names and history files.
>>>          </para>
>>>         </listitem>
>>>        </varlistentry>
>>
>> Maybe this could be fixed instead?
> 
> Indeed, and strtoul is probably a better option than sscanf, don't you 
> think ?

Yeah, the use of sscanf() is kind of weird here.  We have been moving 
the option parsing to use option_parse_int().  Maybe hex support could 
be added there.  Or just use strtoul().

  16. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-03-07T17:14:58Z

    On 06/03/2023 18:04, Peter Eisentraut wrote:
> On 03.03.23 16:52, Sébastien Lardière wrote:
>> On 02/03/2023 09:12, Peter Eisentraut wrote:
>>>
>>> I think here it would be more helpful to show actual examples. Like, 
>>> here is a possible file name, this is what the different parts mean.
>>
>> So you mean explain the WAL filename and the history filename ? Is it 
>> the good place for it ?
>
> Well, your patch says, by the way, the timeline ID in the file is 
> hexadecimal.  Then one might ask, what file, what is a timeline, what 
> are the other numbers in the file, etc.  It seems very specific in 
> this context.  I don't know if the format of these file names is 
> actually documented somewhere.


Well, in the context of this patch, the usage both filename are 
explained juste before, so it seems understandable to me

Timelines are explained in this place : 
https://www.postgresql.org/docs/current/continuous-archiving.html#BACKUP-TIMELINES 
so the patch explains the format there


>
>>>>
>>>
>>> This applies to all configuration parameters, so it doesn't need to 
>>> be mentioned explicitly for individual ones.
>>
>> Probably, but is there another parameter with the same consequence ?
>>
>> worth it to document this point globally ?
>
> It's ok to mention it again.  We do something similar for example at 
> unix_socket_permissions.  But maybe with more context, like "If you 
> want to specify a timeline ID hexadecimal (for example, if extracted 
> from a WAL file name), then prefix it with a 0x".


Ok, I've improved the message


>
>>>
>>> Maybe this could be fixed instead?
>>
>> Indeed, and strtoul is probably a better option than sscanf, don't 
>> you think ?
>
> Yeah, the use of sscanf() is kind of weird here.  We have been moving 
> the option parsing to use option_parse_int().  Maybe hex support could 
> be added there.  Or just use strtoul().


I've made the change with strtoul

About option_parse_int(), actually, strtoint() is used, do we need a 
option_parse_ul() fonction ?

patch attached,

best regards,


-- 
Sébastien

  17. Re: Timeline ID hexadecimal format

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-20T08:17:27Z

    I have committed the two documentation changes, with some minor adjustments.

On 07.03.23 18:14, Sébastien Lardière wrote:
>>>> Maybe this could be fixed instead?
>>>
>>> Indeed, and strtoul is probably a better option than sscanf, don't 
>>> you think ?
>>
>> Yeah, the use of sscanf() is kind of weird here.  We have been moving 
>> the option parsing to use option_parse_int().  Maybe hex support could 
>> be added there.  Or just use strtoul().
> 
> 
> I've made the change with strtoul
> 
> About option_parse_int(), actually, strtoint() is used, do we need a 
> option_parse_ul() fonction ?

For the option parsing, I propose the attached patch.  This follows the 
structure of option_parse_int(), so in the future it could be extracted 
and refactored in the same way, if there is more need.

  18. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-03-20T09:40:31Z

    On 20/03/2023 09:17, Peter Eisentraut wrote:
> I have committed the two documentation changes, with some minor 
> adjustments.


Thank you,


>
> On 07.03.23 18:14, Sébastien Lardière wrote:
>>>>> Maybe this could be fixed instead?
>>>>
>>>> Indeed, and strtoul is probably a better option than sscanf, don't 
>>>> you think ?
>>>
>>> Yeah, the use of sscanf() is kind of weird here.  We have been 
>>> moving the option parsing to use option_parse_int().  Maybe hex 
>>> support could be added there.  Or just use strtoul().
>>
>>
>> I've made the change with strtoul
>>
>> About option_parse_int(), actually, strtoint() is used, do we need a 
>> option_parse_ul() fonction ?
>
> For the option parsing, I propose the attached patch.  This follows 
> the structure of option_parse_int(), so in the future it could be 
> extracted and refactored in the same way, if there is more need.


ok for me, it accept 0x values and refuse wrong values

thank you,


-- 
Sébastien

  19. Re: Timeline ID hexadecimal format

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-21T07:15:22Z

    On 20.03.23 10:40, Sébastien Lardière wrote:
>>> About option_parse_int(), actually, strtoint() is used, do we need a 
>>> option_parse_ul() fonction ?
>>
>> For the option parsing, I propose the attached patch.  This follows 
>> the structure of option_parse_int(), so in the future it could be 
>> extracted and refactored in the same way, if there is more need.
> 
> 
> ok for me, it accept 0x values and refuse wrong values

committed

  20. Re: Timeline ID hexadecimal format

    Sébastien Lardière <sebastien@lardiere.net> — 2023-03-21T07:37:45Z

    On 21/03/2023 08:15, Peter Eisentraut wrote:
> On 20.03.23 10:40, Sébastien Lardière wrote:
>>>> About option_parse_int(), actually, strtoint() is used, do we need 
>>>> a option_parse_ul() fonction ?
>>>
>>> For the option parsing, I propose the attached patch.  This follows 
>>> the structure of option_parse_int(), so in the future it could be 
>>> extracted and refactored in the same way, if there is more need.
>>
>>
>> ok for me, it accept 0x values and refuse wrong values
>
> committed
>
thanks,


-- 
Sébastien