[RFC/PATCH] Implement new '--output-dir' option.

[Sergio Durigan Junior]

On Saturday, March 22 2014, Thadeu Lima de Souza Cascardo wrote:

>> -static void save_rec_file(char *cpf, char *buffer, int len)
>> +static void save_rec_file(char *cpf, char *buffer, int len, const struct rnetclient_args *args)
>>  {
>> -	int fd;
>> -	char *filename;
>> -	char *home, *tmpdir;
>> -	mode_t mask;
>> -	size_t fnlen;
>> -	int r;
>> -	home = getenv("HOME");
>> -	if (!home) {
>> -		tmpdir = getenv("TMPDIR");
>> -		if (!tmpdir)
>> -			tmpdir = "/tmp";
>> -		home = tmpdir;
>> +	FILE *f;
>> +	char cwd[PATH_MAX];
>> +	char *path, *fname, *tmp;
>> +	size_t fname_len, r;
>> +	mode_t old_mask;
>> +	/* If the user provided the output directory where she wishes
>> +	   to save the receipt, then we use it.  Otherwise, we save
>> +	   the file in the current working directory (CWD).  */
>> +	if (args->output_dir == NULL)
>> +		path = getcwd(cwd, PATH_MAX);
>
> By default, I would rather use the same directory as the input file. But
> it could be an incremental patch.

I think it is OK to use CWD for now.  IMO the important thing is not to
use $HOME always.  As you said, we could implement this later.

>> +	else {
>> +		struct stat st;
>> +		if (stat(args->output_dir, &st) < 0) {
>> +			fprintf(stderr, "Could not stat directory \"%s\": %s\n", args->output_dir, strerror(errno));
>> +			return;
>> +		}
>> +		if (!S_ISDIR(st.st_mode)) {
>> +			fprintf(stderr, "Error: \"%s\" is a not a directory.\n", args->output_dir);
>> +			return;
>> +		}
>> +		path = args->output_dir;
>>  	}
>> -	fnlen = strlen(home) + strlen(cpf) + 13;
>> -	filename = malloc(fnlen);
>> -	snprintf(filename, fnlen, "%s/%s.REC.XXXXXX", home, cpf);
>> -	mask = umask(0177);
>> -	fd = mkstemp(filename);
>
> From mkstemp manpage:
> The file is opened with the open(2) O_EXCL flag, guaranteeing that the
> caller is the process that creates the file.
>
> The call to fopen below will overwrite any existing files. Please, open
> it with O_EXCL. Optionally, use a program option for the user to
> explictly allow overwrites, and remove the flag in that case.

That was on purpose.  I wanted the file to be overwritten.  But thinking
about it again, O_EXCL seems a better approach.  I will replace the call
to fopen and use open instead.

OTOH, I don't think it's worth including an option just to allow
overwrites.  I won't do that now.

> This new option could be an incremental patch, but the default behavior
> should not overwrite any files.

OK, fair enough.

> The last incremental patch that I would like to see in this series is
> open the file before connecting to the server, just in case it can't be
> written to, we advise the user and do not send the report.

This is strongly related to what I wrote in the commit message.  I am
not happy with the way we are dealing with the files right now, but I
didn't want to touch this problem in this patch.  I intend to send
another patch later to fix that, so for now I will resubmit this patch
with the s/fopen/open modification.  Does that sound OK to you?

BTW, I think it is strange to talk in english about a program made for
brazilians :-P.

-- 
Sergio