All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH: TinyCompress] fix error reporting in tinycompress
@ 2013-05-23  7:45 Vinod Koul
  2013-05-26 14:13 ` Vinod Koul
  0 siblings, 1 reply; 2+ messages in thread
From: Vinod Koul @ 2013-05-23  7:45 UTC (permalink / raw)
  To: alsa-devel; +Cc: Vinod Koul, Lakshmi N Vinnakota

From: Lakshmi N Vinnakota <lakshmi.n.vinnakota@intel.com>

The compress API are expected to return a negative value on an error
with errno indicating the type of error. But in some cases, this is
not happening.

The fix is the oops function always returns -1 and the actual
error number is determined by the global variable errno.

Signed-off-by: Lakshmi N Vinnakota <lakshmi.n.vinnakota@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 compress.c |   61 ++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/compress.c b/compress.c
index 8753758..7f56c78 100644
--- a/compress.c
+++ b/compress.c
@@ -102,10 +102,11 @@ static int oops(struct compress *compress, int e, const char *fmt, ...)
 	va_end(ap);
 	sz = strlen(compress->error);
 
-	if (errno)
-		snprintf(compress->error + sz, COMPR_ERR_MAX - sz,
-			": %s", strerror(e));
-	return e;
+	snprintf(compress->error + sz, COMPR_ERR_MAX - sz,
+		": %s", strerror(e));
+	errno = e;
+
+	return -1;
 }
 
 const char *compress_get_error(struct compress *compress)
@@ -151,27 +152,27 @@ static bool _is_codec_supported(struct compress *compress, struct compr_config *
 		}
 	}
 	if (codec == false) {
-		oops(compress, -ENXIO, "this codec is not supported");
+		oops(compress, ENXIO, "this codec is not supported");
 		return false;
 	}
 
 	if (config->fragment_size < caps->min_fragment_size) {
-		oops(compress, -EINVAL, "requested fragment size %d is below min supported %d",
+		oops(compress, EINVAL, "requested fragment size %d is below min supported %d",
 			config->fragment_size, caps->min_fragment_size);
 		return false;
 	}
 	if (config->fragment_size > caps->max_fragment_size) {
-		oops(compress, -EINVAL, "requested fragment size %d is above max supported %d",
+		oops(compress, EINVAL, "requested fragment size %d is above max supported %d",
 			config->fragment_size, caps->max_fragment_size);
 		return false;
 	}
 	if (config->fragments < caps->min_fragments) {
-		oops(compress, -EINVAL, "requested fragments %d are below min supported %d",
+		oops(compress, EINVAL, "requested fragments %d are below min supported %d",
 			config->fragments, caps->min_fragments);
 		return false;
 	}
 	if (config->fragments > caps->max_fragments) {
-		oops(compress, -EINVAL, "requested fragments %d are above max supported %d",
+		oops(compress, EINVAL, "requested fragments %d are above max supported %d",
 			config->fragments, caps->max_fragments);
 		return false;
 	}
@@ -219,7 +220,7 @@ struct compress *compress_open(unsigned int card, unsigned int device,
 	char fn[256];
 
 	if (!config) {
-		oops(&bad_compress, -EINVAL, "passed bad config");
+		oops(&bad_compress, EINVAL, "passed bad config");
 		return &bad_compress;
 	}
 
@@ -242,7 +243,7 @@ struct compress *compress_open(unsigned int card, unsigned int device,
 
 	compress->flags = flags;
 	if (!((flags & COMPRESS_OUT) || (flags & COMPRESS_IN))) {
-		oops(&bad_compress, -EINVAL, "can't deduce device direction from given flags");
+		oops(&bad_compress, EINVAL, "can't deduce device direction from given flags");
 		goto config_fail;
 	}
 
@@ -315,12 +316,12 @@ int compress_get_hpointer(struct compress *compress,
 	__u64 time;
 
 	if (!is_compress_ready(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 
 	if (ioctl(compress->fd, SNDRV_COMPRESS_AVAIL, &kavail))
 		return oops(compress, errno, "cannot get avail");
 	if (0 == kavail.tstamp.sampling_rate)
-		return oops(compress, -ENODATA, "sample rate unknown");
+		return oops(compress, ENODATA, "sample rate unknown");
 	*avail = (unsigned int)kavail.avail;
 	time = kavail.tstamp.pcm_io_frames / kavail.tstamp.sampling_rate;
 	tstamp->tv_sec = time;
@@ -335,7 +336,7 @@ int compress_get_tstamp(struct compress *compress,
 	struct snd_compr_tstamp ktstamp;
 
 	if (!is_compress_ready(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 
 	if (ioctl(compress->fd, SNDRV_COMPRESS_TSTAMP, &ktstamp))
 		return oops(compress, errno, "cannot get tstamp");
@@ -355,9 +356,9 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size
 	const unsigned int frag_size = compress->config->fragment_size;
 
 	if (!(compress->flags & COMPRESS_IN))
-		return oops(compress, -EINVAL, "Invalid flag set");
+		return oops(compress, EINVAL, "Invalid flag set");
 	if (!is_compress_ready(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 	fds.fd = compress->fd;
 	fds.events = POLLOUT;
 
@@ -383,7 +384,7 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size
 				continue;
 			}
 			if (fds.revents & POLLERR) {
-				return oops(compress, -EIO, "poll returned error!");
+				return oops(compress, EIO, "poll returned error!");
 			}
 		}
 		/* write avail bytes */
@@ -415,9 +416,9 @@ int compress_read(struct compress *compress, void *buf, unsigned int size)
 	const unsigned int frag_size = compress->config->fragment_size;
 
 	if (!(compress->flags & COMPRESS_OUT))
-		return oops(compress, -EINVAL, "Invalid flag set");
+		return oops(compress, EINVAL, "Invalid flag set");
 	if (!is_compress_ready(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 	fds.fd = compress->fd;
 	fds.events = POLLIN;
 
@@ -440,7 +441,7 @@ int compress_read(struct compress *compress, void *buf, unsigned int size)
 				continue;
 			}
 			if (fds.revents & POLLERR) {
-				return oops(compress, -EIO, "poll returned error!");
+				return oops(compress, EIO, "poll returned error!");
 			}
 		}
 		/* read avail bytes */
@@ -466,7 +467,7 @@ int compress_read(struct compress *compress, void *buf, unsigned int size)
 int compress_start(struct compress *compress)
 {
 	if (!is_compress_ready(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 	if (ioctl(compress->fd, SNDRV_COMPRESS_START))
 		return oops(compress, errno, "cannot start the stream");
 	compress->running = 1;
@@ -477,7 +478,7 @@ int compress_start(struct compress *compress)
 int compress_stop(struct compress *compress)
 {
 	if (!is_compress_running(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 	if (ioctl(compress->fd, SNDRV_COMPRESS_STOP))
 		return oops(compress, errno, "cannot stop the stream");
 	return 0;
@@ -486,7 +487,7 @@ int compress_stop(struct compress *compress)
 int compress_pause(struct compress *compress)
 {
 	if (!is_compress_running(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 	if (ioctl(compress->fd, SNDRV_COMPRESS_PAUSE))
 		return oops(compress, errno, "cannot pause the stream");
 	return 0;
@@ -502,7 +503,7 @@ int compress_resume(struct compress *compress)
 int compress_drain(struct compress *compress)
 {
 	if (!is_compress_running(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 	if (ioctl(compress->fd, SNDRV_COMPRESS_DRAIN))
 		return oops(compress, errno, "cannot drain the stream");
 	return 0;
@@ -511,10 +512,10 @@ int compress_drain(struct compress *compress)
 int compress_partial_drain(struct compress *compress)
 {
 	if (!is_compress_running(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 
 	if (!compress->next_track)
-		return oops(compress, -EPERM, "next track not signalled");
+		return oops(compress, EPERM, "next track not signalled");
 	if (ioctl(compress->fd, SNDRV_COMPRESS_PARTIAL_DRAIN))
 		return oops(compress, errno, "cannot drain the stream\n");
 	compress->next_track = 0;
@@ -524,10 +525,10 @@ int compress_partial_drain(struct compress *compress)
 int compress_next_track(struct compress *compress)
 {
 	if (!is_compress_running(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 
 	if (!compress->gapless_metadata)
-		return oops(compress, -EPERM, "metadata not set");
+		return oops(compress, EPERM, "metadata not set");
 	if (ioctl(compress->fd, SNDRV_COMPRESS_NEXT_TRACK))
 		return oops(compress, errno, "cannot set next track\n");
 	compress->next_track = 1;
@@ -542,14 +543,14 @@ int compress_set_gapless_metadata(struct compress *compress,
 	int version;
 
 	if (!is_compress_ready(compress))
-		return oops(compress, -ENODEV, "device not ready");
+		return oops(compress, ENODEV, "device not ready");
 
 	version = get_compress_version(compress);
 	if (version <= 0)
 		return -1;
 
 	if (version < SNDRV_PROTOCOL_VERSION(0, 1, 1))
-		return oops(compress, -ENXIO, "gapless apis not supported in kernel");
+		return oops(compress, ENXIO, "gapless apis not supported in kernel");
 
 	metadata.key = SNDRV_COMPRESS_ENCODER_PADDING;
 	metadata.value[0] = mdata->encoder_padding;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH: TinyCompress] fix error reporting in tinycompress
  2013-05-23  7:45 [PATCH: TinyCompress] fix error reporting in tinycompress Vinod Koul
@ 2013-05-26 14:13 ` Vinod Koul
  0 siblings, 0 replies; 2+ messages in thread
From: Vinod Koul @ 2013-05-26 14:13 UTC (permalink / raw)
  To: alsa-devel; +Cc: Lakshmi N Vinnakota

On Thu, May 23, 2013 at 01:15:19PM +0530, Vinod Koul wrote:
> From: Lakshmi N Vinnakota <lakshmi.n.vinnakota@intel.com>
> 
> The compress API are expected to return a negative value on an error
> with errno indicating the type of error. But in some cases, this is
> not happening.
> 
> The fix is the oops function always returns -1 and the actual
> error number is determined by the global variable errno.
> 
> Signed-off-by: Lakshmi N Vinnakota <lakshmi.n.vinnakota@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Applied, Thanks

--
~Vinod
> ---
>  compress.c |   61 ++++++++++++++++++++++++++++++-----------------------------
>  1 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/compress.c b/compress.c
> index 8753758..7f56c78 100644
> --- a/compress.c
> +++ b/compress.c
> @@ -102,10 +102,11 @@ static int oops(struct compress *compress, int e, const char *fmt, ...)
>  	va_end(ap);
>  	sz = strlen(compress->error);
>  
> -	if (errno)
> -		snprintf(compress->error + sz, COMPR_ERR_MAX - sz,
> -			": %s", strerror(e));
> -	return e;
> +	snprintf(compress->error + sz, COMPR_ERR_MAX - sz,
> +		": %s", strerror(e));
> +	errno = e;
> +
> +	return -1;
>  }
>  
>  const char *compress_get_error(struct compress *compress)
> @@ -151,27 +152,27 @@ static bool _is_codec_supported(struct compress *compress, struct compr_config *
>  		}
>  	}
>  	if (codec == false) {
> -		oops(compress, -ENXIO, "this codec is not supported");
> +		oops(compress, ENXIO, "this codec is not supported");
>  		return false;
>  	}
>  
>  	if (config->fragment_size < caps->min_fragment_size) {
> -		oops(compress, -EINVAL, "requested fragment size %d is below min supported %d",
> +		oops(compress, EINVAL, "requested fragment size %d is below min supported %d",
>  			config->fragment_size, caps->min_fragment_size);
>  		return false;
>  	}
>  	if (config->fragment_size > caps->max_fragment_size) {
> -		oops(compress, -EINVAL, "requested fragment size %d is above max supported %d",
> +		oops(compress, EINVAL, "requested fragment size %d is above max supported %d",
>  			config->fragment_size, caps->max_fragment_size);
>  		return false;
>  	}
>  	if (config->fragments < caps->min_fragments) {
> -		oops(compress, -EINVAL, "requested fragments %d are below min supported %d",
> +		oops(compress, EINVAL, "requested fragments %d are below min supported %d",
>  			config->fragments, caps->min_fragments);
>  		return false;
>  	}
>  	if (config->fragments > caps->max_fragments) {
> -		oops(compress, -EINVAL, "requested fragments %d are above max supported %d",
> +		oops(compress, EINVAL, "requested fragments %d are above max supported %d",
>  			config->fragments, caps->max_fragments);
>  		return false;
>  	}
> @@ -219,7 +220,7 @@ struct compress *compress_open(unsigned int card, unsigned int device,
>  	char fn[256];
>  
>  	if (!config) {
> -		oops(&bad_compress, -EINVAL, "passed bad config");
> +		oops(&bad_compress, EINVAL, "passed bad config");
>  		return &bad_compress;
>  	}
>  
> @@ -242,7 +243,7 @@ struct compress *compress_open(unsigned int card, unsigned int device,
>  
>  	compress->flags = flags;
>  	if (!((flags & COMPRESS_OUT) || (flags & COMPRESS_IN))) {
> -		oops(&bad_compress, -EINVAL, "can't deduce device direction from given flags");
> +		oops(&bad_compress, EINVAL, "can't deduce device direction from given flags");
>  		goto config_fail;
>  	}
>  
> @@ -315,12 +316,12 @@ int compress_get_hpointer(struct compress *compress,
>  	__u64 time;
>  
>  	if (!is_compress_ready(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_AVAIL, &kavail))
>  		return oops(compress, errno, "cannot get avail");
>  	if (0 == kavail.tstamp.sampling_rate)
> -		return oops(compress, -ENODATA, "sample rate unknown");
> +		return oops(compress, ENODATA, "sample rate unknown");
>  	*avail = (unsigned int)kavail.avail;
>  	time = kavail.tstamp.pcm_io_frames / kavail.tstamp.sampling_rate;
>  	tstamp->tv_sec = time;
> @@ -335,7 +336,7 @@ int compress_get_tstamp(struct compress *compress,
>  	struct snd_compr_tstamp ktstamp;
>  
>  	if (!is_compress_ready(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_TSTAMP, &ktstamp))
>  		return oops(compress, errno, "cannot get tstamp");
> @@ -355,9 +356,9 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size
>  	const unsigned int frag_size = compress->config->fragment_size;
>  
>  	if (!(compress->flags & COMPRESS_IN))
> -		return oops(compress, -EINVAL, "Invalid flag set");
> +		return oops(compress, EINVAL, "Invalid flag set");
>  	if (!is_compress_ready(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  	fds.fd = compress->fd;
>  	fds.events = POLLOUT;
>  
> @@ -383,7 +384,7 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size
>  				continue;
>  			}
>  			if (fds.revents & POLLERR) {
> -				return oops(compress, -EIO, "poll returned error!");
> +				return oops(compress, EIO, "poll returned error!");
>  			}
>  		}
>  		/* write avail bytes */
> @@ -415,9 +416,9 @@ int compress_read(struct compress *compress, void *buf, unsigned int size)
>  	const unsigned int frag_size = compress->config->fragment_size;
>  
>  	if (!(compress->flags & COMPRESS_OUT))
> -		return oops(compress, -EINVAL, "Invalid flag set");
> +		return oops(compress, EINVAL, "Invalid flag set");
>  	if (!is_compress_ready(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  	fds.fd = compress->fd;
>  	fds.events = POLLIN;
>  
> @@ -440,7 +441,7 @@ int compress_read(struct compress *compress, void *buf, unsigned int size)
>  				continue;
>  			}
>  			if (fds.revents & POLLERR) {
> -				return oops(compress, -EIO, "poll returned error!");
> +				return oops(compress, EIO, "poll returned error!");
>  			}
>  		}
>  		/* read avail bytes */
> @@ -466,7 +467,7 @@ int compress_read(struct compress *compress, void *buf, unsigned int size)
>  int compress_start(struct compress *compress)
>  {
>  	if (!is_compress_ready(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_START))
>  		return oops(compress, errno, "cannot start the stream");
>  	compress->running = 1;
> @@ -477,7 +478,7 @@ int compress_start(struct compress *compress)
>  int compress_stop(struct compress *compress)
>  {
>  	if (!is_compress_running(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_STOP))
>  		return oops(compress, errno, "cannot stop the stream");
>  	return 0;
> @@ -486,7 +487,7 @@ int compress_stop(struct compress *compress)
>  int compress_pause(struct compress *compress)
>  {
>  	if (!is_compress_running(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_PAUSE))
>  		return oops(compress, errno, "cannot pause the stream");
>  	return 0;
> @@ -502,7 +503,7 @@ int compress_resume(struct compress *compress)
>  int compress_drain(struct compress *compress)
>  {
>  	if (!is_compress_running(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_DRAIN))
>  		return oops(compress, errno, "cannot drain the stream");
>  	return 0;
> @@ -511,10 +512,10 @@ int compress_drain(struct compress *compress)
>  int compress_partial_drain(struct compress *compress)
>  {
>  	if (!is_compress_running(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  
>  	if (!compress->next_track)
> -		return oops(compress, -EPERM, "next track not signalled");
> +		return oops(compress, EPERM, "next track not signalled");
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_PARTIAL_DRAIN))
>  		return oops(compress, errno, "cannot drain the stream\n");
>  	compress->next_track = 0;
> @@ -524,10 +525,10 @@ int compress_partial_drain(struct compress *compress)
>  int compress_next_track(struct compress *compress)
>  {
>  	if (!is_compress_running(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  
>  	if (!compress->gapless_metadata)
> -		return oops(compress, -EPERM, "metadata not set");
> +		return oops(compress, EPERM, "metadata not set");
>  	if (ioctl(compress->fd, SNDRV_COMPRESS_NEXT_TRACK))
>  		return oops(compress, errno, "cannot set next track\n");
>  	compress->next_track = 1;
> @@ -542,14 +543,14 @@ int compress_set_gapless_metadata(struct compress *compress,
>  	int version;
>  
>  	if (!is_compress_ready(compress))
> -		return oops(compress, -ENODEV, "device not ready");
> +		return oops(compress, ENODEV, "device not ready");
>  
>  	version = get_compress_version(compress);
>  	if (version <= 0)
>  		return -1;
>  
>  	if (version < SNDRV_PROTOCOL_VERSION(0, 1, 1))
> -		return oops(compress, -ENXIO, "gapless apis not supported in kernel");
> +		return oops(compress, ENXIO, "gapless apis not supported in kernel");
>  
>  	metadata.key = SNDRV_COMPRESS_ENCODER_PADDING;
>  	metadata.value[0] = mdata->encoder_padding;
> -- 
> 1.7.0.4
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-05-26 14:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-23  7:45 [PATCH: TinyCompress] fix error reporting in tinycompress Vinod Koul
2013-05-26 14:13 ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.