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

2014-11-02 Thread Lukasz Marek 
On 2 November 2014 00:02, Reynaldo H. Verdejo Pinochet 
reyna...@osg.samsung.com wrote:



 On 11/01/2014 07:59 PM, Lukasz Marek wrote:
  [..]
 
  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. :)
 

 Fair enough. We can work on those limits latter on.

 
  OK, will do that locally.
 

 Thanks. Feel free to push afterward.


Pushed
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2014-11-01 Thread Reynaldo H. Verdejo Pinochet 
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?

  config-nb_max_http_connections = val;
 +if (config-nb_max_connections  config-nb_max_http_connections)
 +ERROR(Inconsistent configuration: MaxClients(%d)  
 MaxHTTPConnections(%d)\n,
 +  config-nb_max_connections, 
 config-nb_max_http_connections);
  } else if (!av_strcasecmp(cmd, MaxClients)) {
  ffserver_get_arg(arg, sizeof(arg), p);
 -val = atoi(arg);
 -if (val  1 || val  config-nb_max_http_connections)
 -ERROR(Invalid MaxClients: %s\n, arg);
 -else
 -config-nb_max_connections = val;
 +ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, 
 Invalid MaxClients: %s\n, arg);
 +config-nb_max_connections = val;
 +if (config-nb_max_connections  config-nb_max_http_connections)
 +ERROR(Inconsistent configuration: MaxClients(%d)  
 MaxHTTPConnections(%d)\n,
 +  config-nb_max_connections, 
 config-nb_max_http_connections);

Same as above

 [...]
 @@ -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()

Rest looks OK.
Bests,

-- 
Reynaldo H. Verdejo Pinochet
Open Source Group
Samsung Research America / Silicon Valley
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2014-11-01 Thread Lukasz Marek 

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.


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2014-11-01 Thread Reynaldo H. Verdejo Pinochet 


On 11/01/2014 07:59 PM, Lukasz Marek wrote:
 [..]
 
 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. :)
 

Fair enough. We can work on those limits latter on.

 
 OK, will do that locally.
 

Thanks. Feel free to push afterward.

Bests,

-- 
Reynaldo H. Verdejo Pinochet
Open Source Group
Samsung Research America / Silicon Valley
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2014-10-31 Thread Lukasz Marek 
Replace atoi with more advanced parsing routine.
Set maximum port value to 65535 (not 65536).
Other checks.

Signed-off-by: Lukasz Marek lukasz.m.lu...@gmail.com
---
 ffserver_config.c | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/ffserver_config.c b/ffserver_config.c
index 6989bd8..c694934 100644
--- a/ffserver_config.c
+++ b/ffserver_config.c
@@ -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);
 config-nb_max_http_connections = val;
+if (config-nb_max_connections  config-nb_max_http_connections)
+ERROR(Inconsistent configuration: MaxClients(%d)  
MaxHTTPConnections(%d)\n,
+  config-nb_max_connections, config-nb_max_http_connections);
 } else if (!av_strcasecmp(cmd, MaxClients)) {
 ffserver_get_arg(arg, sizeof(arg), p);
-val = atoi(arg);
-if (val  1 || val  config-nb_max_http_connections)
-ERROR(Invalid MaxClients: %s\n, arg);
-else
-config-nb_max_connections = val;
+ffserver_set_int_param(val, arg, 0, 1, 65535, config, line_num, 
Invalid MaxClients: %s\n, arg);
+config-nb_max_connections = val;
+if (config-nb_max_connections  config-nb_max_http_connections)
+ERROR(Inconsistent configuration: MaxClients(%d)  
MaxHTTPConnections(%d)\n,
+  config-nb_max_connections, config-nb_max_http_connections);
 } else if (!av_strcasecmp(cmd, MaxBandwidth)) {
 int64_t llval;
+char *tailp;
 ffserver_get_arg(arg, sizeof(arg), p);
-llval = strtoll(arg, NULL, 10);
-if (llval  10 || llval  1000)
+errno = 0;
+llval = strtoll(arg, tailp, 10);
+if (llval  10 || llval  1000 || tailp[0] || errno)
 ERROR(Invalid MaxBandwidth: %s\n, arg);
 else
 config-max_bandwidth = llval;
@@ -405,7 +404,7 @@ static int ffserver_parse_config_global(FFServerConfig 
*config, const char *cmd,
 if (!config-debug)
 ffserver_get_arg(config-logfilename, sizeof(config-logfilename), 
p);
 } else if (!av_strcasecmp(cmd, LoadModule)) {
-ERROR(Loadable modules no longer supported\n);
+ERROR(Loadable modules are no longer supported\n);
 } else
 ERROR(Incorrect keyword: '%s'\n, cmd);
 return 0;
@@ -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;