[media-ctl,PATCHv5,1/5] libmediactl: restruct error path

Commit Message

Andy Shevchenko Sept. 5, 2011, 3:24 p.m. UTC

Signed-off-by: Andy Shevchenko <[e-mail]>
---
 src/media.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

Comments

Laurent Pinchart Sept. 6, 2011, 10:25 a.m. UTC | #1

Hi Andy,

Thank you for the patches.

I've slightly modified 1/5 and 3/5 (the first one returned -1 from 
media_enum_entities(), which made media-ctl stop with a failure message) and 
pushed the result to the repository.

I've also added another patch to fix autoconf malloc/realloc tests when cross-
compiling that resulted in a compilation failure.

Andy Shevchenko Sept. 6, 2011, 10:46 a.m. UTC | #2

On Tue, 2011-09-06 at 12:25 +0200, Laurent Pinchart wrote: 
> I've slightly modified 1/5 and 3/5 (the first one returned -1 from 
> media_enum_entities(), which made media-ctl stop with a failure message) and 
> pushed the result to the repository.
Okay. I looked at them.
One minor comment: udef_unref is aware of NULL.

Andy Shevchenko Sept. 6, 2011, 10:50 a.m. UTC | #3

On Tue, 2011-09-06 at 13:46 +0300, Andy Shevchenko wrote: 
> On Tue, 2011-09-06 at 12:25 +0200, Laurent Pinchart wrote: 
> > I've slightly modified 1/5 and 3/5 (the first one returned -1 from 
> > media_enum_entities(), which made media-ctl stop with a failure message) and 
> > pushed the result to the repository.
> Okay. I looked at them.
> One minor comment: udef_unref is aware of NULL.
Ah, and another. I don't get why you split snprintf() to that suboptimal
strncpy + x[sizeof(x)-1] = 0?

Laurent Pinchart Sept. 6, 2011, 11:30 a.m. UTC | #4

Hi Andy,

On Tuesday 06 September 2011 12:50:25 Andy Shevchenko wrote:
> On Tue, 2011-09-06 at 13:46 +0300, Andy Shevchenko wrote:
> > On Tue, 2011-09-06 at 12:25 +0200, Laurent Pinchart wrote:
> > > I've slightly modified 1/5 and 3/5 (the first one returned -1 from
> > > media_enum_entities(), which made media-ctl stop with a failure
> > > message) and pushed the result to the repository.
> > 
> > Okay. I looked at them.
> > One minor comment: udef_unref is aware of NULL.

I wasn't aware of that, thanks.

> Ah, and another. I don't get why you split snprintf() to that suboptimal
> strncpy + x[sizeof(x)-1] = 0?

snprintf needs to parse the format argument, is strncpy really suboptimal ?

Patch

diff --git a/src/media.c b/src/media.c
index e3cab86..050289e 100644
--- a/src/media.c
+++ b/src/media.c
@@ -255,7 +255,7 @@  static int media_enum_entities(struct media_device *media)
 	char target[1024];
 	char *p;
 	__u32 id;
-	int ret;
+	int ret = 0;
 
 	for (id = 0; ; id = entity->info.id) {
 		size = (media->entities_count + 1) * sizeof(*media->entities);
@@ -268,9 +268,9 @@  static int media_enum_entities(struct media_device *media)
 
 		ret = ioctl(media->fd, MEDIA_IOC_ENUM_ENTITIES, &entity->info);
 		if (ret < 0) {
-			if (errno == EINVAL)
-				break;
-			return -errno;
+			if (errno != EINVAL)
+				ret = -errno;
+			break;
 		}
 
 		/* Number of links (for outbound links) plus number of pads (for
@@ -281,8 +281,10 @@  static int media_enum_entities(struct media_device *media)
 
 		entity->pads = malloc(entity->info.pads * sizeof(*entity->pads));
 		entity->links = malloc(entity->max_links * sizeof(*entity->links));
-		if (entity->pads == NULL || entity->links == NULL)
-			return -ENOMEM;
+		if (entity->pads == NULL || entity->links == NULL) {
+			ret = -ENOMEM;
+			break;
+		}
 
 		media->entities_count++;
 
@@ -316,7 +318,7 @@  static int media_enum_entities(struct media_device *media)
 			strcpy(entity->devname, devname);
 	}
 
-	return 0;
+	return ret;
 }
 
 struct media_device *media_open(const char *name, int verbose)