[FFmpeg-devel] [PATCH 3/4] ffserver_config: improve error handling

Lukasz Marek lukasz.m.luki2 at gmail.com
Sat Nov 1 23:59:52 CET 2014


On 01.11.2014 23:06, Reynaldo H. Verdejo Pinochet wrote:
> Hi
>
> On 10/31/2014 11:00 PM, Lukasz Marek wrote:
>> [..]
>> @@ -356,9 +356,7 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd,
>>           if (!av_strcasecmp(cmd, "Port"))
>>               WARNING("Port option is deprecated, use HTTPPort instead\n");
>>           ffserver_get_arg(arg, sizeof(arg), p);
>> -        val = atoi(arg);
>> -        if (val < 1 || val > 65536)
>> -            ERROR("Invalid port: %s\n", arg);
>> +        ffserver_set_int_param(&val, arg, 0, 1, 65535, config, line_num, "Invalid port: %s\n", arg);
>>           if (val < 1024)
>>               WARNING("Trying to use IETF assigned system port: %d\n", val);
>>           config->http_addr.sin_port = htons(val);
>> @@ -367,37 +365,38 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd,
>>               WARNING("BindAddress option is deprecated, use HTTPBindAddress instead\n");
>>           ffserver_get_arg(arg, sizeof(arg), p);
>>           if (resolve_host(&config->http_addr.sin_addr, arg) != 0)
>> -            ERROR("%s:%d: Invalid host/IP address: %s\n", arg);
>> +            ERROR("Invalid host/IP address: %s\n", arg);
>>       } else if (!av_strcasecmp(cmd, "NoDaemon")) {
>>           WARNING("NoDaemon option has no effect, you should remove it\n");
>>       } else if (!av_strcasecmp(cmd, "RTSPPort")) {
>>           ffserver_get_arg(arg, sizeof(arg), p);
>> -        val = atoi(arg);
>> -        if (val < 1 || val > 65536)
>> -            ERROR("%s:%d: Invalid port: %s\n", arg);
>> -        config->rtsp_addr.sin_port = htons(atoi(arg));
>> +        ffserver_set_int_param(&val, arg, 0, 1, 65535, config, line_num, "Invalid port: %s\n", arg);
>> +        config->rtsp_addr.sin_port = htons(val);
>>       } else if (!av_strcasecmp(cmd, "RTSPBindAddress")) {
>>           ffserver_get_arg(arg, sizeof(arg), p);
>>           if (resolve_host(&config->rtsp_addr.sin_addr, arg) != 0)
>>               ERROR("Invalid host/IP address: %s\n", arg);
>>       } else if (!av_strcasecmp(cmd, "MaxHTTPConnections")) {
>>           ffserver_get_arg(arg, sizeof(arg), p);
>> -        val = atoi(arg);
>> -        if (val < 1 || val > 65536)
>> -            ERROR("Invalid MaxHTTPConnections: %s\n", arg);
>> +        ffserver_set_int_param(&val, arg, 0, 1, 65535, config, line_num, "Invalid MaxHTTPConnections: %s\n", arg);
>
> I don't think we should be capping MaxHTTPConnections at 65535.
> If there's a reason then FFServerConfig.nb_max_http_connections
> type needs to be revised too?

I decreased it by 1 automatically rather than for any reason. I didn't 
want to change logic where it was not needed, and it was not needed 
here. I guess you ask in general if there should be a limit (not just it 
is 65535 or 6). I don't know, but as I stated before, I don't want to 
change the limit, just int parsing routine. If you wish I can change 
back to 65536 or to INT_MAX, but I want it to be clear it is not my 
original intention. :)

>> [...]
>> @@ -500,6 +499,9 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd, c
>>           case 'G':
>>               fsize *= 1024 * 1024 * 1024;
>>               break;
>> +        default:
>> +            ERROR("Invalid file size: %s\n", arg);
>> +            break;
>>           }
>>           feed->feed_max_size = (int64_t)fsize;
>>           if (feed->feed_max_size < FFM_PACKET_SIZE*4)
>> @@ -876,11 +878,15 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd,
>>           stream->is_multicast = 1;
>>           stream->loop = 1; /* default is looping */
>>       } else if (!av_strcasecmp(cmd, "MulticastPort")) {
>> +        int val;
>>           ffserver_get_arg(arg, sizeof(arg), p);
>> -        stream->multicast_port = atoi(arg);
>> +        ffserver_set_int_param(&val, arg, 0, 1, 65535, config, line_num, "Invalid MulticastPort: %s\n", arg);
>> +        stream->multicast_port = val;
>>       } else if (!av_strcasecmp(cmd, "MulticastTTL")) {
>> +        int val;
>
> Maybe declare val once at the beginning
> of ffserver_parse_config_stream()

OK, will do that locally.




More information about the ffmpeg-devel mailing list