Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1192457 > unrolled thread

[PATCH] userns: simplify map_id_range_* functions

Back to article view | Back to linux.kernel

Contents

  [PATCH] userns: simplify map_id_range_* functions Nicolas Iooss <nicolas.iooss_linux@m4x.org> - 2015-07-26 04:10 +0200
    Re: [PATCH] userns: simplify map_id_range_* functions ebiederm@xmission.com (Eric W. Biederman) - 2015-07-27 06:40 +0200

#1192457 — [PATCH] userns: simplify map_id_range_* functions

Functions map_id_range_down, map_id_down and map_id_up all used the
construction:

    if (...)
        id = ...
    else
        id = ...
    return id;

which can be simplified by directly returning the result of the
computations in each branch.

Moreover as the condition tested whether the "break;" in the previous
for loop was hit, it is simpler to directly compute the result and
return it.

Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
---

As I could not find any relevant entry for kernel/user_namespace.c in
MAINTAINERS, I have built the list of recipients from the git history.
Please let me know if I was expected to proceed differently to submit
this patch.

 kernel/user_namespace.c | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 4109f8320684..bf063dc6b8d4 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -165,15 +165,10 @@ static u32 map_id_range_down(struct uid_gid_map *map, u32 id, u32 count)
 		last = first + map->extent[idx].count - 1;
 		if (id >= first && id <= last &&
 		    (id2 >= first && id2 <= last))
-			break;
+			return (id - first) + map->extent[idx].lower_first;
 	}
-	/* Map the id or note failure */
-	if (idx < extents)
-		id = (id - first) + map->extent[idx].lower_first;
-	else
-		id = (u32) -1;
-
-	return id;
+	/* Note failure */
+	return (u32) -1;
 }
 
 static u32 map_id_down(struct uid_gid_map *map, u32 id)
@@ -188,15 +183,9 @@ static u32 map_id_down(struct uid_gid_map *map, u32 id)
 		first = map->extent[idx].first;
 		last = first + map->extent[idx].count - 1;
 		if (id >= first && id <= last)
-			break;
+			return (id - first) + map->extent[idx].lower_first;
 	}
-	/* Map the id or note failure */
-	if (idx < extents)
-		id = (id - first) + map->extent[idx].lower_first;
-	else
-		id = (u32) -1;
-
-	return id;
+	return (u32) -1;
 }
 
 static u32 map_id_up(struct uid_gid_map *map, u32 id)
@@ -211,15 +200,9 @@ static u32 map_id_up(struct uid_gid_map *map, u32 id)
 		first = map->extent[idx].lower_first;
 		last = first + map->extent[idx].count - 1;
 		if (id >= first && id <= last)
-			break;
+			return (id - first) + map->extent[idx].first;
 	}
-	/* Map the id or note failure */
-	if (idx < extents)
-		id = (id - first) + map->extent[idx].first;
-	else
-		id = (u32) -1;
-
-	return id;
+	return (u32) -1;
 }
 
 /**
-- 
2.4.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1192657

Nicolas Iooss <nicolas.iooss_linux@m4x.org> writes:

> Functions map_id_range_down, map_id_down and map_id_up all used the
> construction:
>
>     if (...)
>         id = ...
>     else
>         id = ...
>     return id;
>
> which can be simplified by directly returning the result of the
> computations in each branch.
>
> Moreover as the condition tested whether the "break;" in the previous
> for loop was hit, it is simpler to directly compute the result and
> return it.

It is not a simplification, it is just code motion.

Further at least to my eyes adding multiple exit points and setting the
same value in two different places actually obscures what the functions
are doing.

If we could talk about speeding up the performance of the stat system
call I think there would be a point in mucking with these functions.

As it is I think it is I think merging your patch will just make it more
difficult to understand what the code is doing in the future, with no 
benefit except a reduction in line count.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web