From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups Date: Sun, 08 Nov 2009 10:05:03 -0600 Message-ID: <1257696303.4184.8.camel@mulgrave.site> References: Reply-To: Linux filesystem caching discussion list Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cachefs-bounces@redhat.com Errors-To: linux-cachefs-bounces@redhat.com To: device-mapper development Cc: Pavel Roskin , Stefan Haberland , Jan Kara , linux-cachefs@redhat.com, Mike Snitzer , Neil Brown , Frederic Weisbecker , Jens Axboe , Heiko Carstens , "James E . J . Bottomley" , ibm-acpi-devel@lists.sourceforge.net, Chr, Julia Lawall , "H . Peter Anvin" , Daire Byrne , Alasdair G Kergon , Greg Banks , Stefan Weinhuber , Eric Sandeen , Adam Belay , netfilter-devel@vger.kernel.org, Helge Deller , x86@kernel.org, James Morris , Takashi Iwai , Ingo Molnar , Alan Cox List-Id: linux-raid.ids On Sat, 2009-11-07 at 13:16 -0200, Andr=C3=A9 Goddard Rosa wrote: > This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4= .4.1 > even considering that it exports a newly defined function skip_spaces() > to drivers: > text data bss dec hex filename = =20 > 64867 840 592 66299 102fb (TOTALS-lib.a-before) > 64954 584 588 66126 1024e (TOTALS-lib.a-after) > and implements some code tidy up. >=20 > Besides reducing lib.a size, it converts many in-tree drivers to use th= e > newly defined function, which makes another small reduction on kernel s= ize > overall when those drivers are used. Before we embark on something as massive as this, could we take a step back. I agree that if I were coming up with the strstip() interface today I probably wouldn't have given it two overloaded uses. However, I think the function, in spite of this minor issue, is very usable. I still don't understand why people thought adding a __must_check, which is what damaged one of the overloaded uses, is a good idea. Assuming there's a good answer to the above: > + * skip_spaces - Removes leading whitespace from @s. > + * @s: The string to be stripped. > + * > + * Returns a pointer to the first non-whitespace character in @s. > + */ > +const char *skip_spaces(const char *str) I don't think const return is a good idea because most functions will be manipulating the string and using pointers that won't be const, so this will generate a ton of 'initialization discards qualifiers from pointer target type' ... so that leads to the question of whether this patch series was actually compiled ... James From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups Date: Sun, 08 Nov 2009 10:05:03 -0600 Message-ID: <1257696303.4184.8.camel@mulgrave.site> References: Reply-To: Linux filesystem caching discussion list Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Pavel Roskin , Stefan Haberland , Jan Kara , linux-cachefs@redhat.com, Mike Snitzer , Neil Brown , Frederic Weisbecker , Jens Axboe , Heiko Carstens , "James E . J . Bottomley" , ibm-acpi-devel@lists.sourceforge.net, Chr, Julia Lawall , "H . Peter Anvin" , Daire Byrne , Alasdair G Kergon , Greg Banks , Stefan Weinhuber , Eric Sandeen , Adam Belay , netfilter-devel@vger.kernel.org, Helge Deller , x86@kernel.org, James Morris , Takashi Iwai , Ingo Molnar , Alan Cox Return-path: In-Reply-To: List-Id: Linux filesystem caching discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-cachefs-bounces@redhat.com On Sat, 2009-11-07 at 13:16 -0200, Andr=C3=A9 Goddard Rosa wrote: > This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4= .4.1 > even considering that it exports a newly defined function skip_spaces() > to drivers: > text data bss dec hex filename = =20 > 64867 840 592 66299 102fb (TOTALS-lib.a-before) > 64954 584 588 66126 1024e (TOTALS-lib.a-after) > and implements some code tidy up. >=20 > Besides reducing lib.a size, it converts many in-tree drivers to use th= e > newly defined function, which makes another small reduction on kernel s= ize > overall when those drivers are used. Before we embark on something as massive as this, could we take a step back. I agree that if I were coming up with the strstip() interface today I probably wouldn't have given it two overloaded uses. However, I think the function, in spite of this minor issue, is very usable. I still don't understand why people thought adding a __must_check, which is what damaged one of the overloaded uses, is a good idea. Assuming there's a good answer to the above: > + * skip_spaces - Removes leading whitespace from @s. > + * @s: The string to be stripped. > + * > + * Returns a pointer to the first non-whitespace character in @s. > + */ > +const char *skip_spaces(const char *str) I don't think const return is a good idea because most functions will be manipulating the string and using pointers that won't be const, so this will generate a ton of 'initialization discards qualifiers from pointer target type' ... so that leads to the question of whether this patch series was actually compiled ... James From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups Date: Sun, 08 Nov 2009 10:05:03 -0600 Message-ID: <1257696303.4184.8.camel@mulgrave.site> References: Reply-To: Linux filesystem caching discussion list Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: Pavel Roskin , Stefan Haberland , Jan Kara , linux-cachefs@redhat.com, Mike Snitzer , Neil Brown , Frederic Weisbecker , Jens Axboe , Heiko Carstens , "James E . J . Bottomley" , ibm-acpi-devel@lists.sourceforge.net, Chr, Julia Lawall , "H . Peter Anvin" , Daire Byrne , Alasdair G Kergon , Greg Banks , Stefan Weinhuber , Eric Sandeen , Adam Belay , netfilter-devel@vger.kernel.org, Helge Deller , x86@kernel.org, James Morris , Takashi Iwai , Ingo Molnar , Alan Cox Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cachefs-bounces@redhat.com Errors-To: linux-cachefs-bounces@redhat.com List-Id: netdev.vger.kernel.org On Sat, 2009-11-07 at 13:16 -0200, Andr=C3=A9 Goddard Rosa wrote: > This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4= .4.1 > even considering that it exports a newly defined function skip_spaces() > to drivers: > text data bss dec hex filename = =20 > 64867 840 592 66299 102fb (TOTALS-lib.a-before) > 64954 584 588 66126 1024e (TOTALS-lib.a-after) > and implements some code tidy up. >=20 > Besides reducing lib.a size, it converts many in-tree drivers to use th= e > newly defined function, which makes another small reduction on kernel s= ize > overall when those drivers are used. Before we embark on something as massive as this, could we take a step back. I agree that if I were coming up with the strstip() interface today I probably wouldn't have given it two overloaded uses. However, I think the function, in spite of this minor issue, is very usable. I still don't understand why people thought adding a __must_check, which is what damaged one of the overloaded uses, is a good idea. Assuming there's a good answer to the above: > + * skip_spaces - Removes leading whitespace from @s. > + * @s: The string to be stripped. > + * > + * Returns a pointer to the first non-whitespace character in @s. > + */ > +const char *skip_spaces(const char *str) I don't think const return is a good idea because most functions will be manipulating the string and using pointers that won't be const, so this will generate a ton of 'initialization discards qualifiers from pointer target type' ... so that leads to the question of whether this patch series was actually compiled ... James From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: James Bottomley In-Reply-To: References: Date: Sun, 08 Nov 2009 10:05:03 -0600 Message-Id: <1257696303.4184.8.camel@mulgrave.site> Mime-Version: 1.0 Subject: Re: [uml-devel] [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups List-Id: The user-mode Linux development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: user-mode-linux-devel-bounces@lists.sourceforge.net To: device-mapper development Cc: Pavel Roskin , Stefan Haberland , Jan Kara , linux-cachefs@redhat.com, Mike Snitzer , Neil Brown , Frederic Weisbecker , Jens Axboe , Heiko Carstens , Steve Dickson , "James E . J . Bottomley" , David Howells , ibm-acpi-devel@lists.sourceforge.net, Chr@nerdnet.nl, Julia Lawall , "H . Peter Anvin" , Daire Byrne , Alasdair G Kergon , Greg Banks , Stefan Weinhuber , Eric Sandeen , Adam Belay , netfilter-devel@vger.kernel.org, Helge Deller , x86@kernel.org, James Morris , Takashi Iwai , Ingo Molnar , Alan Cox , WANG Cong , Roman Hoog Antink , linux-ext4@vger.kernel.org, Alexey Dobriyan , Andrea Righi , alsa-devel@alsa-project.org, Len Brown , Samuel Ortiz , coreteam@netfilter.org, user-mode-linux-devel@lists.sourceforge.net, linux-s390@vger.kernel.org, Trond Myklebust , Jeff Dike , Rusty Russell , Henrique de Moraes Holschuh , Kyle@nerdnet.nl, Steven Rostedt , Sitsofe Wheeler , linux-raid@vger.kernel.org, Martin Schwidefsky , Andreas Dilger , Pekka Enberg , Mikulas Patocka , Al Viro , user-mode-linux-user@lists.sourceforge.net, Jaroslav Kysela , Thomas Gleixner , Michael Holzheu , Arjan van de Ven , Jan Engelhardt , Bjorn Helgaas , Andre Noll , Theodore Ts'o , linux-parisc@vger.kernel.org, "Martin K . Petersen" , =?ISO-8859-1?Q?Andr=E9?= Goddard Rosa , netdev@vger.kernel.org, Jason Baron , Greg Kroah-Hartman , Roel Kluin , linux-kernel@vger.kernel.org, Stoyan Gaydarov , Patrick McHardy , n , istof Schmitt , netfilter@vger.kernel.org, Richard Purdie , Joe Perches , linux390@de.ibm.com, Andrew Morton , Andrey Borzenkov , "David S . Miller" T24gU2F0LCAyMDA5LTExLTA3IGF0IDEzOjE2IC0wMjAwLCBBbmRyw6kgR29kZGFyZCBSb3NhIHdy b3RlOgo+IFRoaXMgcGF0Y2ggcmVkdWNlcyBsaWIuYSBjb2RlIHNpemUgYnkgMTczIGJ5dGVzIG9u IG15IENvcmUgMiB3aXRoIGdjYyA0LjQuMQo+IGV2ZW4gY29uc2lkZXJpbmcgdGhhdCBpdCBleHBv cnRzIGEgbmV3bHkgZGVmaW5lZCBmdW5jdGlvbiBza2lwX3NwYWNlcygpCj4gdG8gZHJpdmVyczoK PiAgICB0ZXh0ICAgIGRhdGEgICAgIGJzcyAgICAgZGVjICAgICBoZXggZmlsZW5hbWUgICAgICAg ICAgICAgICAgICAgICAgICAgICAKPiAgIDY0ODY3ICAgICA4NDAgICAgIDU5MiAgIDY2Mjk5ICAg MTAyZmIgKFRPVEFMUy1saWIuYS1iZWZvcmUpCj4gICA2NDk1NCAgICAgNTg0ICAgICA1ODggICA2 NjEyNiAgIDEwMjRlIChUT1RBTFMtbGliLmEtYWZ0ZXIpCj4gYW5kIGltcGxlbWVudHMgc29tZSBj b2RlIHRpZHkgdXAuCj4gCj4gQmVzaWRlcyByZWR1Y2luZyBsaWIuYSBzaXplLCBpdCBjb252ZXJ0 cyBtYW55IGluLXRyZWUgZHJpdmVycyB0byB1c2UgdGhlCj4gbmV3bHkgZGVmaW5lZCBmdW5jdGlv biwgd2hpY2ggbWFrZXMgYW5vdGhlciBzbWFsbCByZWR1Y3Rpb24gb24ga2VybmVsIHNpemUKPiBv dmVyYWxsIHdoZW4gdGhvc2UgZHJpdmVycyBhcmUgdXNlZC4KCkJlZm9yZSB3ZSBlbWJhcmsgb24g c29tZXRoaW5nIGFzIG1hc3NpdmUgYXMgdGhpcywgY291bGQgd2UgdGFrZSBhIHN0ZXAKYmFjay4g IEkgYWdyZWUgdGhhdCBpZiBJIHdlcmUgY29taW5nIHVwIHdpdGggdGhlIHN0cnN0aXAoKSBpbnRl cmZhY2UKdG9kYXkgSSBwcm9iYWJseSB3b3VsZG4ndCBoYXZlIGdpdmVuIGl0IHR3byBvdmVybG9h ZGVkIHVzZXMuCgpIb3dldmVyLCBJIHRoaW5rIHRoZSBmdW5jdGlvbiwgaW4gc3BpdGUgb2YgdGhp cyBtaW5vciBpc3N1ZSwgaXMgdmVyeQp1c2FibGUuICBJIHN0aWxsIGRvbid0IHVuZGVyc3RhbmQg d2h5IHBlb3BsZSB0aG91Z2h0IGFkZGluZyBhCl9fbXVzdF9jaGVjaywgd2hpY2ggaXMgd2hhdCBk YW1hZ2VkIG9uZSBvZiB0aGUgb3ZlcmxvYWRlZCB1c2VzLCBpcyBhCmdvb2QgaWRlYS4KCkFzc3Vt aW5nIHRoZXJlJ3MgYSBnb29kIGFuc3dlciB0byB0aGUgYWJvdmU6Cgo+ICsgKiBza2lwX3NwYWNl cyAtIFJlbW92ZXMgbGVhZGluZyB3aGl0ZXNwYWNlIGZyb20gQHMuCj4gKyAqIEBzOiBUaGUgc3Ry aW5nIHRvIGJlIHN0cmlwcGVkLgo+ICsgKgo+ICsgKiBSZXR1cm5zIGEgcG9pbnRlciB0byB0aGUg Zmlyc3Qgbm9uLXdoaXRlc3BhY2UgY2hhcmFjdGVyIGluIEBzLgo+ICsgKi8KPiArY29uc3QgY2hh ciAqc2tpcF9zcGFjZXMoY29uc3QgY2hhciAqc3RyKQoKSSBkb24ndCB0aGluayBjb25zdCByZXR1 cm4gaXMgYSBnb29kIGlkZWEgYmVjYXVzZSBtb3N0IGZ1bmN0aW9ucyB3aWxsIGJlCm1hbmlwdWxh dGluZyB0aGUgc3RyaW5nIGFuZCB1c2luZyBwb2ludGVycyB0aGF0IHdvbid0IGJlIGNvbnN0LCBz byB0aGlzCndpbGwgZ2VuZXJhdGUgYSB0b24gb2YgJ2luaXRpYWxpemF0aW9uIGRpc2NhcmRzIHF1 YWxpZmllcnMgZnJvbSBwb2ludGVyCnRhcmdldCB0eXBlJyAuLi4gc28gdGhhdCBsZWFkcyB0byB0 aGUgcXVlc3Rpb24gb2Ygd2hldGhlciB0aGlzIHBhdGNoCnNlcmllcyB3YXMgYWN0dWFsbHkgY29t cGlsZWQgLi4uCgpKYW1lcwoKCgotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KTGV0IENyeXN0YWwgUmVw b3J0cyBoYW5kbGUgdGhlIHJlcG9ydGluZyAtIEZyZWUgQ3J5c3RhbCBSZXBvcnRzIDIwMDggMzAt RGF5IAp0cmlhbC4gU2ltcGxpZnkgeW91ciByZXBvcnQgZGVzaWduLCBpbnRlZ3JhdGlvbiBhbmQg ZGVwbG95bWVudCAtIGFuZCBmb2N1cyBvbiAKd2hhdCB5b3UgZG8gYmVzdCwgY29yZSBhcHBsaWNh dGlvbiBjb2RpbmcuIERpc2NvdmVyIHdoYXQncyBuZXcgd2l0aApDcnlzdGFsIFJlcG9ydHMgbm93 LiAgaHR0cDovL3Auc2YubmV0L3NmdS9ib2JqLWp1bHkKX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KVXNlci1tb2RlLWxpbnV4LWRldmVsIG1haWxpbmcgbGlz dApVc2VyLW1vZGUtbGludXgtZGV2ZWxAbGlzdHMuc291cmNlZm9yZ2UubmV0Cmh0dHBzOi8vbGlz dHMuc291cmNlZm9yZ2UubmV0L2xpc3RzL2xpc3RpbmZvL3VzZXItbW9kZS1saW51eC1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [dm-devel] [PATCH v4 00/12] introduce skip_spaces(), reducing code size plus some clean-ups Date: Sun, 08 Nov 2009 10:05:03 -0600 Message-ID: <1257696303.4184.8.camel@mulgrave.site> References: Reply-To: Linux filesystem caching discussion list Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cachefs-bounces@redhat.com Errors-To: linux-cachefs-bounces@redhat.com Content-Type: text/plain; charset="iso-8859-1" To: device-mapper development Cc: Pavel Roskin , Stefan Haberland , Jan Kara , linux-cachefs@redhat.com, Mike Snitzer , Neil Brown , Frederic Weisbecker , Jens Axboe , Heiko Carstens , "James E . J . Bottomley" , ibm-acpi-devel@lists.sourceforge.net, Chr, Julia Lawall , "H . Peter Anvin" , Daire Byrne , Alasdair G Kergon , Greg Banks , Stefan Weinhuber , Eric Sandeen , Adam Belay , netfilter-devel@vger.kernel.org, Helge Deller , x86@kernel.org, James Morris , Takashi Iwai , Ingo Molnar , Alan Cox On Sat, 2009-11-07 at 13:16 -0200, Andr=C3=A9 Goddard Rosa wrote: > This patch reduces lib.a code size by 173 bytes on my Core 2 with gcc 4= .4.1 > even considering that it exports a newly defined function skip_spaces() > to drivers: > text data bss dec hex filename = =20 > 64867 840 592 66299 102fb (TOTALS-lib.a-before) > 64954 584 588 66126 1024e (TOTALS-lib.a-after) > and implements some code tidy up. >=20 > Besides reducing lib.a size, it converts many in-tree drivers to use th= e > newly defined function, which makes another small reduction on kernel s= ize > overall when those drivers are used. Before we embark on something as massive as this, could we take a step back. I agree that if I were coming up with the strstip() interface today I probably wouldn't have given it two overloaded uses. However, I think the function, in spite of this minor issue, is very usable. I still don't understand why people thought adding a __must_check, which is what damaged one of the overloaded uses, is a good idea. Assuming there's a good answer to the above: > + * skip_spaces - Removes leading whitespace from @s. > + * @s: The string to be stripped. > + * > + * Returns a pointer to the first non-whitespace character in @s. > + */ > +const char *skip_spaces(const char *str) I don't think const return is a good idea because most functions will be manipulating the string and using pointers that won't be const, so this will generate a ton of 'initialization discards qualifiers from pointer target type' ... so that leads to the question of whether this patch series was actually compiled ... James