Re: psql: \dl+ to list large objects privileges - Mailing list pgsql-hackers

Hi,

On a high level I will recommend the addition of tests. There are similar tests

Tests added.

Applying the patch, generates several whitespace warnings. It will be helpful
if those warnings are removed.

I know this is a silly mistake, and after reading this article[1] I tried to remove the extra spaces.

Can you tell me, please, how can you get such warnings?

The patch contains:
                        case 'l':
-                               success = do_lo_list();
+                               success = listLargeObjects(show_verbose);


It might be of some interest to consider in the above to check the value of the
next character in command or emit an error if not valid. Such a pattern can be
found in the same switch block as for example:
                    switch (cmd[2])                    {                        case '\0':                        case '+':                <snip>                        success = ...                </snip>                            break;                        default:                            status = PSQL_CMD_UNKNOWN;                            break;                    }

Check added.

The patch contains:
                else if (strcmp(cmd + 3, "list") == 0)
-                       success = do_lo_list();
+                       success = listLargeObjects(false);
+
+               else if (strcmp(cmd + 3, "list+") == 0)
+                       success = listLargeObjects(true);


In a fashion similar to `exec_command_list`, it might be interesting to consider
expressing the above as:
        show_verbose = strchr(cmd, '+') ? true : false;        <snip>        else if (strcmp(cmd + 3, "list") == 0                success = do_lo_list(show_verbose);

I rewrote this part.

New version attached.

[1] https://wiki.postgresql.org/wiki/Creating_Clean_Patches

--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
The Russian Postgres Company