[gnutls-devel] GnuTLS | Add an option "--file-log" to redirect some information to a temporary file (!940)

[Development of GNU's TLS library]

Merge request https://gitlab.com/gnutls/gnutls/merge_requests/940 was reviewed by Nathaniel McCallum

--
  
Nathaniel McCallum started a new discussion on src/cli.c:

> +			dup2(oldstdout,STDOUT_FILENO);
> +			close(oldstdout);
> +		}

This `do_once` semantic is unnecessary if you refactor the code as I stated above. Just set the global variable to `NULL` after fclose().

--
  
Nathaniel McCallum started a new discussion on src/cli.c:

> +		oldstdout=dup(STDOUT_FILENO);
> +		dup2(log_file,STDOUT_FILENO);
> +		close(log_file);

This is definitely the wrong approach. Replacing STDOUT, while clever, is going to lead to very bad behavior by the application. You need to use `fopen()` and create a global `FILE` instance, which defaults to `stdout` (the existing behavior). All logging commands then need to be updated to use this new global variable. The patch should be MUCH larger than this.

--
  
Nathaniel McCallum started a new discussion on src/cli.c:

> +		log_file = open("stdout_log.txt",O_WRONLY | O_CREAT);
> +		if(log_file <=0){
> +			fprintf(stderr,"Failed to create a log file!\n The Errno is %m");

Like above, follow the style. Also, your error message needs to be changed. You don't always want to create a file.

--
  
Nathaniel McCallum started a new discussion on src/cli.c:

>  	}
> -
> +	if(HAVE_OPT(FILE_LOG)){

Space after `if`. Space before `{`. You need to make sure you match the existing code style and not impose your own.

--
  
Nathaniel McCallum started a new discussion on src/cli.c:

> -
> +	if(HAVE_OPT(FILE_LOG)){
> +		log_file = open("stdout_log.txt",O_WRONLY | O_CREAT);

We definitely don't want a hard-coded file name. The option should specify the file name. That way we can redirect to files like `/dev/null` or `/dev/stdout`.

Also, I'm pretty sure you want `fopen()` here, not `open()`.

--
  
Nathaniel McCallum started a new discussion on src/cli.c:

>  	int ret;
>  	int ii, inp;
> +	int oldstdout = -1,log_file = -1,do_once=0;

@DK\_DARKmatter Almost every code style (except really weird ones) specify a space after a comma.


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/merge_requests/940
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20190227/d0bef8f8/attachment-0001.html>