| Message ID | 20240822133908.1042240-5-lizetao1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some modifications to optimize code readability | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | fail | "Bluetooth: " prefix is not specified in the subject |
On Thu, Aug 22, 2024 at 09:39:04PM +0800, Li Zetao wrote: > When resolving name in ceph_dns_resolve_name(), the end address of name > is determined by the minimum value of delim_p and colon_p. So using min() > here is more in line with the context. > > Signed-off-by: Li Zetao <lizetao1@huawei.com> > --- > net/ceph/messenger.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > index 3c8b78d9c4d1..d1b5705dc0c6 100644 > --- a/net/ceph/messenger.c > +++ b/net/ceph/messenger.c > @@ -1254,7 +1254,7 @@ static int ceph_dns_resolve_name(const char *name, size_t namelen, > colon_p = memchr(name, ':', namelen); > > if (delim_p && colon_p) > - end = delim_p < colon_p ? delim_p : colon_p; > + end = min(delim_p, colon_p); Both delim_p, and colon_p are char *, so this seems correct to me. And the code being replaced does appear to be a min() operation in both form and function. Reviewed-by: Simon Horman <horms@kernel.org> However, I don't believe libceph changes usually don't go through next-next. So I think this either needs to be reposted or get some acks from one of the maintainers. Ilya, Xiubo, perhaps you can offer some guidance here? > else if (!delim_p && colon_p) > end = colon_p; > else { > -- > 2.34.1 > >
On Sat, Aug 24, 2024 at 8:12 PM Simon Horman <horms@kernel.org> wrote: > > On Thu, Aug 22, 2024 at 09:39:04PM +0800, Li Zetao wrote: > > When resolving name in ceph_dns_resolve_name(), the end address of name > > is determined by the minimum value of delim_p and colon_p. So using min() > > here is more in line with the context. > > > > Signed-off-by: Li Zetao <lizetao1@huawei.com> > > --- > > net/ceph/messenger.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > > index 3c8b78d9c4d1..d1b5705dc0c6 100644 > > --- a/net/ceph/messenger.c > > +++ b/net/ceph/messenger.c > > @@ -1254,7 +1254,7 @@ static int ceph_dns_resolve_name(const char *name, size_t namelen, > > colon_p = memchr(name, ':', namelen); > > > > if (delim_p && colon_p) > > - end = delim_p < colon_p ? delim_p : colon_p; > > + end = min(delim_p, colon_p); > > Both delim_p, and colon_p are char *, so this seems correct to me. > > And the code being replaced does appear to be a min() operation in > both form and function. > > Reviewed-by: Simon Horman <horms@kernel.org> > > However, I don't believe libceph changes usually don't go through next-next. > So I think this either needs to be reposted or get some acks from > one of the maintainers. > > Ilya, Xiubo, perhaps you can offer some guidance here? Hi Simon, I'm OK with this being taken through net-next. Acked-by: Ilya Dryomov <idryomov@gmail.com> Thanks, Ilya
On Sun, Aug 25, 2024 at 6:21 PM Ilya Dryomov <idryomov@gmail.com> wrote: > > On Sat, Aug 24, 2024 at 8:12 PM Simon Horman <horms@kernel.org> wrote: > > > > On Thu, Aug 22, 2024 at 09:39:04PM +0800, Li Zetao wrote: > > > When resolving name in ceph_dns_resolve_name(), the end address of name > > > is determined by the minimum value of delim_p and colon_p. So using min() > > > here is more in line with the context. > > > > > > Signed-off-by: Li Zetao <lizetao1@huawei.com> > > > --- > > > net/ceph/messenger.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > > > index 3c8b78d9c4d1..d1b5705dc0c6 100644 > > > --- a/net/ceph/messenger.c > > > +++ b/net/ceph/messenger.c > > > @@ -1254,7 +1254,7 @@ static int ceph_dns_resolve_name(const char *name, size_t namelen, > > > colon_p = memchr(name, ':', namelen); > > > > > > if (delim_p && colon_p) > > > - end = delim_p < colon_p ? delim_p : colon_p; > > > + end = min(delim_p, colon_p); > > > > Both delim_p, and colon_p are char *, so this seems correct to me. > > > > And the code being replaced does appear to be a min() operation in > > both form and function. > > > > Reviewed-by: Simon Horman <horms@kernel.org> > > > > However, I don't believe libceph changes usually don't go through next-next. > > So I think this either needs to be reposted or get some acks from > > one of the maintainers. > > > > Ilya, Xiubo, perhaps you can offer some guidance here? > > Hi Simon, > > I'm OK with this being taken through net-next. I see that Jakub picked up some patches from this series, but not this one. I'll go ahead and apply to the Ceph tree. Thanks, Ilya
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 3c8b78d9c4d1..d1b5705dc0c6 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -1254,7 +1254,7 @@ static int ceph_dns_resolve_name(const char *name, size_t namelen, colon_p = memchr(name, ':', namelen); if (delim_p && colon_p) - end = delim_p < colon_p ? delim_p : colon_p; + end = min(delim_p, colon_p); else if (!delim_p && colon_p) end = colon_p; else {
When resolving name in ceph_dns_resolve_name(), the end address of name is determined by the minimum value of delim_p and colon_p. So using min() here is more in line with the context. Signed-off-by: Li Zetao <lizetao1@huawei.com> --- net/ceph/messenger.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)