From: Andrey Abramov <st5pub@yandex.ru> To: George Spelvin <lkml@sdf.org>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> Cc: "adrian.hunter@intel.com" <adrian.hunter@intel.com>, "ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>, "benh@kernel.crashing.org" <benh@kernel.crashing.org>, "bp@alien8.de" <bp@alien8.de>, "darrick.wong@oracle.com" <darrick.wong@oracle.com>, "dchinner@redhat.com" <dchinner@redhat.com>, "dedekind1@gmail.com" <dedekind1@gmail.com>, "hpa@zytor.com" <hpa@zytor.com>, "jlbec@evilplan.org" <jlbec@evilplan.org>, "jpoimboe@redhat.com" <jpoimboe@redhat.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-snps-arc@lists.infradead.org" <linux-snps-arc@lists.infradead.org>, "mark@fasheh.com" <mark@fasheh.com>, "mingo@redhat.com" <mingo@redhat.com>, "mpe@ellerman.id.au" <mpe@ellerman.id.au>, "naveen.n.rao@linux.vnet.ibm.com" <naveen.n.rao@linux.vnet.ibm.com>, "paulus@samba.org" <paulus@samba.org>, "richard@nod.at" <richard@nod.at>, "tglx@linutronix.de" <tglx@linutronix.de>, "vgupta@synopsys.com" <vgupta@synopsys.com>, "x86@kernel.org" <x86@kernel.org> Subject: Re: [PATCH 5/5] Lib: sort.h: replace int size with size_t size in the swap function Date: Sun, 31 Mar 2019 10:00:18 +0300 [thread overview] Message-ID: <20434561554015618@myt1-cd60b8ae9bb9.qloud-c.yandex.net> (raw) In-Reply-To: <201903302015.x2UKFnSL003850@sdf.org> 30.03.2019, 23:17, "George Spelvin" <lkml@sdf.org>: > On Sat, 30 Mar 2019 at 19:38:26 +0100 greh k-h wrote; >> On Sat, Mar 30, 2019 at 07:43:53PM +0300, Andrey Abramov wrote: >>> Replace int type with size_t type of the size argument >>> in the swap function, also affect all its dependencies. >> >> This says _what_ the patch does, but it gives no clue as to _why_ you >> are doing this. Neither did your 0/5 patch :( >> >> Why make this change? Nothing afterward depends on it from what I can >> tell, so why is it needed? > > It's just a minor cleanup, making things less surprising for future > programmers. As I wrote in a comment in my patches, using a signed type > for an object size is definitely a wart; ever since C89 it's expected > you'd use size_t for the purpose. > > The connection is that it's a natural consequence of doing a pass over > every call site. > > You're right it could be dropped from the series harmlessly, but it > comes from the same work. But it's all of *three* call sites in the kernel > which are affected. Surely that's not an unreasonable amount of churn > to clean up a wart? George Spelvin is absolutely right: "It's just a minor cleanup, making things less surprising for future programmers." 31.03.2019, 00:51, "George Spelvin" <lkml@sdf.org>: > It was so obvious to me that I didn't question it, but you have a > good point and I'm sure Andrey can clarify. Thanks for the attention! I thought that it is obvious enough (argument called "size" should be of type size_t in the 90% of cases). Should I resend this patch with better explanation "why"? -- With Best Regards, Andrey Abramov
WARNING: multiple messages have this Message-ID (diff)
From: st5pub@yandex.ru (Andrey Abramov) To: linux-snps-arc@lists.infradead.org Subject: [PATCH 5/5] Lib: sort.h: replace int size with size_t size in the swap function Date: Sun, 31 Mar 2019 10:00:18 +0300 [thread overview] Message-ID: <20434561554015618@myt1-cd60b8ae9bb9.qloud-c.yandex.net> (raw) In-Reply-To: <201903302015.x2UKFnSL003850@sdf.org> 30.03.2019, 23:17, "George Spelvin" <lkml at sdf.org>: > On Sat, 30 Mar 2019 at 19:38:26 +0100 greh k-h wrote; >> ?On Sat, Mar 30, 2019@07:43:53PM +0300, Andrey Abramov wrote: >>> ?Replace int type with size_t type of the size argument >>> ?in the swap function, also affect all its dependencies. >> >> ?This says _what_ the patch does, but it gives no clue as to _why_ you >> ?are doing this. Neither did your 0/5 patch :( >> >> ?Why make this change? Nothing afterward depends on it from what I can >> ?tell, so why is it needed? > > It's just a minor cleanup, making things less surprising for future > programmers. As I wrote in a comment in my patches, using a signed type > for an object size is definitely a wart; ever since C89 it's expected > you'd use size_t for the purpose. > > The connection is that it's a natural consequence of doing a pass over > every call site. > > You're right it could be dropped from the series harmlessly, but it > comes from the same work. But it's all of *three* call sites in the kernel > which are affected. Surely that's not an unreasonable amount of churn > to clean up a wart? George Spelvin is absolutely right: "It's just a minor cleanup, making things less surprising for future programmers." 31.03.2019, 00:51, "George Spelvin" <lkml at sdf.org>: > It was so obvious to me that I didn't question it, but you have a > good point and I'm sure Andrey can clarify. Thanks for the attention! I thought that it is obvious enough (argument called "size" should be of type size_t in the 90% of cases). Should I resend this patch with better explanation "why"? -- With Best Regards, Andrey Abramov
next prev parent reply other threads:[~2019-03-31 7:03 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-30 16:37 [PATCH 0/5] simple sort swap function usage improvements Andrey Abramov 2019-03-30 16:37 ` Andrey Abramov 2019-03-30 16:37 ` Andrey Abramov 2019-03-30 16:37 ` Andrey Abramov 2019-03-30 16:40 ` [PATCH 1/5] arch/arc: unwind.c: replace swap function with built-in one Andrey Abramov 2019-03-30 16:40 ` Andrey Abramov 2019-03-30 16:40 ` Andrey Abramov 2019-03-30 16:40 ` Andrey Abramov 2019-03-30 16:41 ` [PATCH 2/5] powerpc: module_[32|64].c: " Andrey Abramov 2019-03-30 16:41 ` Andrey Abramov 2019-03-30 16:41 ` Andrey Abramov 2019-03-30 16:41 ` Andrey Abramov 2019-04-01 10:11 ` Michael Ellerman 2019-04-02 19:11 ` Andrey Abramov 2019-03-30 16:42 ` [PATCH 3/5] ocfs2: dir,refcounttree,xattr: replace swap functions " Andrey Abramov 2019-03-30 16:42 ` [PATCH 3/5] ocfs2: dir, refcounttree, xattr: " Andrey Abramov 2019-03-30 16:42 ` Andrey Abramov 2019-03-30 16:42 ` Andrey Abramov 2019-03-30 16:43 ` [PATCH 4/5] ubifs: find.c: replace swap function " Andrey Abramov 2019-03-30 16:43 ` Andrey Abramov 2019-03-30 16:43 ` Andrey Abramov 2019-03-30 16:43 ` Andrey Abramov 2019-03-30 16:43 ` [PATCH 5/5] Lib: sort.h: replace int size with size_t size in the swap function Andrey Abramov 2019-03-30 16:43 ` Andrey Abramov 2019-03-30 16:43 ` Andrey Abramov 2019-03-30 16:43 ` Andrey Abramov 2019-03-30 18:38 ` gregkh 2019-03-30 18:38 ` gregkh 2019-03-30 18:38 ` gregkh 2019-03-30 20:15 ` George Spelvin 2019-03-30 20:15 ` George Spelvin 2019-03-30 20:24 ` Greg KH 2019-03-30 20:24 ` Greg KH 2019-03-30 21:49 ` George Spelvin 2019-03-30 21:49 ` George Spelvin 2019-03-31 7:00 ` Andrey Abramov [this message] 2019-03-31 7:00 ` Andrey Abramov 2019-03-31 10:54 ` gregkh 2019-03-31 10:54 ` gregkh 2019-04-01 14:47 ` David Laight 2019-04-01 14:47 ` David Laight 2019-04-01 18:01 ` Vineet Gupta 2019-04-01 18:01 ` Vineet Gupta 2019-04-01 18:14 ` Andrey Abramov 2019-04-01 18:14 ` Andrey Abramov 2019-04-01 18:22 ` Vineet Gupta 2019-04-01 18:22 ` Vineet Gupta 2019-03-30 17:16 ` [PATCH 0/5] simple sort swap function usage improvements George Spelvin 2019-03-30 17:16 ` George Spelvin 2019-03-30 17:16 ` George Spelvin 2019-03-30 17:16 ` George Spelvin 2019-04-01 21:08 ` Rasmus Villemoes 2019-03-30 18:32 ` Andy Shevchenko 2019-03-30 18:32 ` Andy Shevchenko 2019-03-30 18:32 ` Andy Shevchenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20434561554015618@myt1-cd60b8ae9bb9.qloud-c.yandex.net \ --to=st5pub@yandex.ru \ --cc=adrian.hunter@intel.com \ --cc=ard.biesheuvel@linaro.org \ --cc=benh@kernel.crashing.org \ --cc=bp@alien8.de \ --cc=darrick.wong@oracle.com \ --cc=dchinner@redhat.com \ --cc=dedekind1@gmail.com \ --cc=gregkh@linuxfoundation.org \ --cc=hpa@zytor.com \ --cc=jlbec@evilplan.org \ --cc=jpoimboe@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-snps-arc@lists.infradead.org \ --cc=lkml@sdf.org \ --cc=mark@fasheh.com \ --cc=mingo@redhat.com \ --cc=mpe@ellerman.id.au \ --cc=naveen.n.rao@linux.vnet.ibm.com \ --cc=paulus@samba.org \ --cc=richard@nod.at \ --cc=tglx@linutronix.de \ --cc=vgupta@synopsys.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.