Unexpectedly creates a file named ./profanity

[mwgamera]

Current Behavior

After running for a while (apparently when the log file reaches the maxsize limit), profanity changes the log file location to profanity in the current working directory.

Expected Behavior

It shouldn't do that.

Steps to Reproduce

0. Ensure ./profanity doesn't exist and . is writable.
1. Run profanity -l DEBUG (but without -f) to make it write a lot of log.
2. Set /log maxsize 64.
3. Do anything DEBUG-log-worthy like issuing any command.
4. Observe ./profanity being created.

Possible Solution

Pass NULL as a second argument of log_init in _rotate_log_file:

profanity/src/log.c

Lines 249 to 260 in 1ec6065

	char* lf = strdup(mainlogfile);
	char* start = strrchr(lf, '/') + 1;
	char* end = strstr(start, ".log");
	*end = '\0';
	log_close();
	rename(log_file, log_file_new);
	log_init(log_get_filter(), start);
	free(lf);

Apparently, the _rotate_log_file function tried to extract user-provided name from currently used mainlogfile and restart logging to the same place after rotation, but currently this is interpreted as a full path instead. As I understand, the log rotation is no longer done with user-provided paths at all so this should be simply skipped altogether now as passing any non-NULL value is interpreted as user-provided. Replacing start with NULL appears to fix it for me.

[raspbeguy]

Can confirm that, with up-to-date master version.

[jubalh]

@mwgamera can you test whether #1563 fixes this?

[mwgamera]

Yes, it fixes it and I can no longer reproduce it on master. But I think it introduces another subtle bug that the file will be rotated at most once because log_file = g_strdup(mainlogfile) is not NULL so log_init will set user_provided_log to TRUE.

[jubalh]

You mean in _rotate_log_file()? That function is only called if user_provided_log is false.

[mwgamera]

Yes, and as _rotate_log_file invokes log_init with non-NULL log_file, it changes the value of user_provided_log from FALSE to TRUE here:

profanity/src/log.c

Lines 152 to 163 in 482f895

	log_init(log_level_t filter, char* log_file)
	{
	level_filter = filter;
	tz = g_time_zone_new_local();
	if (log_file) {
	user_provided_log = TRUE;
	}
	gchar* lf = files_get_log_file(log_file);
	logp = fopen(lf, "a");

Unless I'm missing something, when _rotate_log_file is reached, the actual path returned by files_get_log_file(NULL) should be the same as current mainlogfile. The only difference is that non-NULL log_file also sets user_provided_log to TRUE disabling further rotations.

[jubalh]

Ah yes, so it should actually pass NULL. Do you want to make a PR?

[mwgamera]

No, please handle it in whatever way you like.