[Icecast-dev] [patch] Reopen dumpfiles on signal

[Ken Treis]

On Feb 25, 2011, at 1:43 AM, Niv Sardi wrote:

> A bit of nitpiking:
> 
> * indentation seems off.

What's the convention for this project? It looked like 4 spaces, and I tried to match but might have let some tabs slip through. Someday I should learn how to tweak vim to do this for me.

>> --- src/source.c	(revision 17873)
>> +++ src/source.c	(working copy)
>> @@ -714,6 +715,20 @@
>>             }
>> 
>>             /* save stream to file */
>> +			if (source->reopen_dumpfile) {
>> +			    if (source->dumpfile) {
>> +					fclose(source->dumpfile);
>> +					source->dumpfile = NULL;
>> +				}
> 
> * here you close a file, then check if there is a ->dumpfilename after
>  you try to open a new one... if there is no ->dumpfilename (for whatever
>  reason, is it possible ?) I'd say the best is not to close the first one.

It's possible if the config is changed and reloaded via HUP. The dumpfile configuration parameter could be removed, and normally the file wouldn't be closed until the source disconnected. With this change, a subsequent USR1 will cause the file to be closed (bringing things into line with the new config). 

The code also handles the opposite situation (B), where a dumpfile config parameter is added to a mount that didn't have one previously.

To me, this all argues for the reopen of dumpfiles to be included in the HUP handler. We're just activating these config parameters immediately instead of waiting for the source to dis/re-connect.

> 
>> +				if (source->dumpfilename) {
>> +                    source->dumpfile = fopen(source->dumpfilename, "ab");
>> +                    if (source->dumpfile == NULL) {
>> +                        WARN2("Cannot re-open dump file \"%s\" for appending: %s, disabling.",
>> +                                source->dumpfilename, strerror(errno));
>> +                    }
>> +				}
>> +                source->reopen_dumpfile = 0;
>> +			}
>>             if (source->dumpfile && source->format->write_buf_to_file)
> 
> * and so with the coments above, you can do your magic here.

If I moved the reopen code here, I'd lose the added benefit (B) above.

> 
>>                 source->format->write_buf_to_file (source, refbuf);
>>         }
> 
> Cheers,
> -- 
> Niv Sardi

--
Ken Treis
Miriam Technologies, Inc.