[systemd-devel] [PATCH] journalctl: barrow logic from pager.c so that journalctl man page stays correct

[Zbigniew Jędrzejewski-Szmek]

On 07/20/2012 09:06 AM, Zbigniew Jedrzejewski-Szmek wrote:
> On Fri, Jul 20, 2012 at 05:42:44AM +0000, Shawn Landen wrote:
>> particularly: "Pager to use when --no-pager is not given; overrides $PAGER.
>>   Setting this to an empty string or the value cat is equivalent to passing --no-pager."
> Hi,
> your description of the problem is valid, but I think that the patch
> could be improved. We don't want to replicate the variable parsing logic
> in two places...
> 
> Can you check if the following works for you:
Commited this fix as fafb6ecc.

Zbyszek

> -----8<--------
> diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> index e9810c9..e633dd3 100644
> --- a/src/journal/journalctl.c
> +++ b/src/journal/journalctl.c
> @@ -398,11 +398,7 @@ int main(int argc, char *argv[]) {
>                  goto finish;
>          }
>  
> -        have_pager = !arg_no_pager && !arg_follow;
> -        if (have_pager) {
> -                columns();
> -                pager_open();
> -        }
> +        have_pager = !arg_no_pager && !arg_follow && pager_open();
>  
>          if (arg_output == OUTPUT_JSON) {
>                  fputc('[', stdout);
> diff --git a/src/shared/pager.c b/src/shared/pager.c
> index 6a85af3..36b409c 100644
> --- a/src/shared/pager.c
> +++ b/src/shared/pager.c
> @@ -44,20 +44,20 @@ _noreturn_ static void pager_fallback(void) {
>          _exit(EXIT_SUCCESS);
>  }
>  
> -void pager_open(void) {
> +bool pager_open(void) {
>          int fd[2];
>          const char *pager;
>          pid_t parent_pid;
>  
>          if (pager_pid > 0)
> -                return;
> +                return false;
>  
>          if ((pager = getenv("SYSTEMD_PAGER")) || (pager = getenv("PAGER")))
>                  if (!*pager || streq(pager, "cat"))
> -                        return;
> +                        return false;
>  
>          if (isatty(STDOUT_FILENO) <= 0)
> -                return;
> +                return false;
>  
>          /* Determine and cache number of columns before we spawn the
>           * pager so that we get the value from the actual tty */
> @@ -65,7 +65,7 @@ void pager_open(void) {
>  
>          if (pipe(fd) < 0) {
>                  log_error("Failed to create pager pipe: %m");
> -                return;
> +                return false;
>          }
>  
>          parent_pid = getpid();
> @@ -74,7 +74,7 @@ void pager_open(void) {
>          if (pager_pid < 0) {
>                  log_error("Failed to fork pager: %m");
>                  close_pipe(fd);
> -                return;
> +                return false;
>          }
>  
>          /* In the child start the pager */
> @@ -115,10 +115,13 @@ void pager_open(void) {
>          }
>  
>          /* Return in the parent */
> -        if (dup2(fd[1], STDOUT_FILENO) < 0)
> +        if (dup2(fd[1], STDOUT_FILENO) < 0) {
>                  log_error("Failed to duplicate pager pipe: %m");
> +                return false;
> +        }
>  
>          close_pipe(fd);
> +        return true;
>  }
>  
>  void pager_close(void) {
> diff --git a/src/shared/pager.h b/src/shared/pager.h
> index dcd6195..0b4afc0 100644
> --- a/src/shared/pager.h
> +++ b/src/shared/pager.h
> @@ -21,5 +21,7 @@
>    along with systemd; If not, see <http://www.gnu.org/licenses/>.
>  ***/
>  
> -void pager_open(void);
> +#include <stdbool.h>
> +
> +bool pager_open(void);
>  void pager_close(void);