"Tsunakawa, Takayuki" <tsunakawa.ta...@jp.fujitsu.com> writes:
> I added a few ereport() calls that emit the same message except for the Win32 
> API name.  Which of the following do you think is the best?  I'd like to 
> follow the majority.

> [Option 1]
>     ereport(elevel,
>         (errmsg("could not enable Lock pages in memory user right"),
>         errdetail("Failed system call was %s, error code %lu", 
> "OpenProcessToken", GetLastError())));

> [Option 2]
>     ereport(elevel,
>         (errmsg("could not enable Lock Pages in Memory user right: error code 
> %lu", GetLastError()),
>         errdetail("Failed system call was OpenProcessToken.")));

TBH, I think you are worried about the wrong thing here.  You could drop
both of those errdetail calls altogether and be little worse off.  In the
places where we have errdetail calls like "failed system call was xxx",
the main point is to show the exact parameters that were given to the
system call, and neither of these do that.  These errdetail messages
would only be useful if the identical ereport errmsg might be issued
for failures from different underlying Windows calls --- but I doubt
that's what you're intending here.

My problem with these messages is I am not sure what "memory user right"
means.  Probably that just needs a bit of editing.  But I'd go for
something like "could not do xxx: error code %lu", and not bother
mentioning the system call name, unless failing to do so has some
impact on whether we could understand what happened from a field
report of this error message.

(See the "Function Names" item in our message style guidelines
for more about this issue.  Maybe we need to expand that item
some more.)

                        regards, tom lane

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to