pg_listen: trigger a shell command on a Postgres event

13

pg_listen: trigger a shell command on a Postgres event ☶ show c databases github.com
authored by begriffs 5 months ago | cached | 10 comments

10

This utility can also be accomplished as almost a one-liner in some high level languages, but my goal was to see how efficient this tool could be. Thus I wrote it in C and worked directly with libpq. Code reviews are welcome since I’m a newbie with low-level programming.

3
friendlysock edited 5 months ago | link
Two things:
- consider using more const char * for functions that do not mutate their arguments
- consider passing the extra field of the pgNotify object as an argument to the shell command (so you can send message) more details here
Note that you probably want to write the payload to a temporary file and spool it to the shell command instead of inserting it directly, because of security concerns.

EDIT:

For the payload thing, maybe also consider fork and execlp for passing args.
1. 3
  pstef 5 months ago | link
  consider using more const char * for functions that do not mutate their arguments
  
  I don’t want to pick nits, but since people may learn from this comment, I’d argue for going full char const * const, like this:
  
  -void listen_forever(PGconn *, const char *, const char *); +void listen_forever(PGconn *, char const * const, char const * const);
  
  I also can’t find a good reason for BUFSZ to be a file scope int. Usually it’d be defined with a #define. Used like that it has a side effect of the char sql[7 + BUFSZ + 1]; becoming a VLA unless gcc decides otherwise.
  1. 2
    
    peter edited 5 months ago | link
    
    Neither of those are necessary. The * const is trivially inferable and no optimizing C compiler worth anything will generate a variable length array from a const int. BUFSZ may be completely eliminated by the link time optimizer as well, since this module will have an initial executable storage class.
    
    cc: @friendlysock
    1. 1
      
      pstef 5 months ago | link
      
      The * const is trivially inferable
      
      In a simple file like this it probably is trivial, but in larger projects I find it easier to follow the code when both pointers and objects pointed to are const-qualified (assuming neither should be modified by the program).
      1. 1
        
        peter 5 months ago | link
        
        I agree it’s useful as documentation to the developer, and as a safeguard against developer mistakes. But it doesn’t help the optimizer. And the last const qualifier on a pointer is local to the function, so it’s only useful for larger / more convoluted functions. I think any time the developer can easily see whether the pointer could be labeled const, it’s unnecessary. If it’s not easy to see, and the function can’t be quickly refactored to make it easier to see, then yeah throw a const on it.
  2. 1
    
    friendlysock 5 months ago | link
    
    Good point! I’m a little lazy on const stuff sometimes. :(
2. 1
  
  begriffs 5 months ago | link
  
  Thanks for the tips. I changed the functions to using const char * for their arguments. For the second point, this program doesn’t actually pass the payload to the command it executes. That’s all the functionality I needed for my own use case but I can see how sending the payload to the command via stdin or something could be useful in certain cases.
1

dbremner 5 months ago | link

You can mark clean_and_die as noreturn. I’m assuming that your compiler supports C11. You will need to include the stdnoreturn.h header.
1. 1
  
  begriffs 5 months ago | link
  
  Cool tip. I also marked listen_forever that way. (I hope C11 support is pretty widespread these days.)
  1. 1
    
    dbremner 5 months ago | link
    
    It is pretty easy to add a macro for systems that don’t support noreturn.
    Here is an example from one of my projects.
    #ifndef NORETURN_H
    #define NORETURN_H
    #ifdef __cplusplus
    #define NORETURN [[noreturn]]
    #else
    #define NORETURN noreturn
    #include <stdnoreturn.h>
    #endif
    #endif