Re: Avoid possible dereference null pointer (src/backend/catalog/pg_depend.c)

On Fri, May 24, 2024 at 11:03 AM Michael Paquier <michael@paquier.xyz>
wrote:

> On Thu, May 23, 2024 at 08:54:12AM -0300, Ranier Vilela wrote:
> > All calls to functions like SearchSysCacheAttName, in the whole codebase,
> > checks if returns are valid.
> > It must be for a very strong reason, such a style.
>
> Usually good practice, as I've outlined once upthread, because we do
> expect the attributes to exist in this case.  Or if you want, an error
> is better than a crash if a concurrent path causes this area to lead
> to inconsistent lookups, which is something I've seen in the past
> while hacking on my own stuff, or just fix other things causing
> syscache lookup inconsistencies.  You'd be surprised to hear that
> dropped attributes being mishandled is not that uncommon, especially
> in out-of-core code, as one example.  FWIW, I don't see much a point
> in using ereport(), the two checks ought to be elog()s pointing to an
> internal error as these two errors should never happen.  Still, it is
> a good idea to check that they never happen: aka an internal
> error state is better than a crash if a problem arises.
>
> > So, v3, implements it this way.
>
> I don't understand the point behind the open/close of attrelation,
> TBH.  That's not needed.
>
> Except fot these two points, this is just moving the calls to make
> sure that we have valid tuples from the syscache, which is a better
> practice.  509199587df7 is recent enough that this should be fixed now
> rather than later.
>

If we are looking for avoiding a segfault and get a message which helps
debugging, using get_attname and get_attnum might be better options.
get_attname throws an error. get_attnum doesn't throw an error and returns
InvalidAttnum which won't return any valid identity sequence, and thus
return a NIL sequence list which is handled in that function already. Using
these two functions will avoid the clutter as well as segfault. If that's
acceptable, I will provide a patch.

-- 
Best Wishes,
Ashutosh Bapat