Re: Enhanced error message to include hint messages for redundant options error

On Sat, May 8, 2021 at 7:06 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Sat, May 8, 2021 at 2:20 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Sat, May 8, 2021 at 12:01 PM houzj.fnst@fujitsu.com
> > <houzj.fnst@fujitsu.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.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com