From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Subject: | Re: Enhanced error message to include hint messages for redundant options error |
Date: | 2021-05-10 13:27:47 |
Message-ID: | CALDaNm2q=1HiDTMHd0B6KVc5_N_HXZpL0eWCU9r2Q1d1EXnAJQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, May 10, 2021 at 6:00 AM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> > > > > > > > Thanks! The v5 patch looks good to me. Let's see if all
> > > > > > > > agree on the goto duplicate_error approach which could reduce
> > the LOC by ~80.
> > > > > > >
> > > > > > > I think the "goto duplicate_error" approach looks good, it
> > > > > > > avoids duplicating the same error code multiple times.
> > > > > >
> > > > > > Thanks. I will mark the v5 patch "ready for committer" if no one has
> > comments.
> > > > >
> > > > > Hi,
> > > > >
> > > > > I looked into the patch and noticed a minor thing.
> > > > >
> > > > > + return; /* keep compiler quiet */
> > > > > }
> > > > >
> > > > > I think we do not need the comment here.
> > > > > The compiler seems not require "return" at the end of function
> > > > > when function's return type is VOID.
> > > > >
> > > > > In addition, it seems better to remove these "return;" like what
> > > > > commit "3974c4" did.
> > > >
> > > > It looks like that commit removed the plain return statements for a
> > > > void returning functions. I see in the code that there are return
> > > > statements that are there right after ereport(ERROR, just to keep
> > > > the compiler quiet. Here in this patch also, we have return;
> > > > statements after ereport(ERROR, for void returning functions. I'm
> > > > not sure removing them would cause some compiler warnings on some
> > > > platforms with some other compilers. If we're not sure, I'm okay to
> > > > keep those return; statements. Thoughts?
> > >
> > > I felt we could retain the return statement and remove the comments.
> > > If you are ok with that I will modify and post a patch for it.
> > > Thoughts?
> >
> > I would like to keep it as is i.e. both return statement and /* keep compiler
> > quiet */ comment. Having said that, it's better to leave it to the committer on
> > whether to have the return statement at all.
>
> Yes, it's better to leave it to the committer on whether to have the "return;".
> But, I think at least removing "return;" which is at the *end* of the function will not cause any warning.
> Such as:
>
> + return; /* keep compiler quiet */
> }
>
> So, I'd vote for at least removing the comment " keep the compiler quiet ".
That sounds fine to me, Attached v6 patch which has the changes for the same.
Regards,
Vignesh
Attachment | Content-Type | Size |
---|---|---|
v6-0001-Enhance-error-message.patch | text/x-patch | 72.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Euler Taveira | 2021-05-10 13:41:24 | Re: row filtering for logical replication |
Previous Message | Michail Nikolaev | 2021-05-10 13:05:09 | Re: [PATCH] Full support for index LP_DEAD hint bits on standby |