Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-04 Thread Edgecombe, Rick P
On Mon, 2024-03-04 at 18:00 +, Christophe Leroy wrote: > > Personally, I think a single patch that sets "= {}" for all of them > > and > > drop the all the "= 0" or "= NULL" assignments would be the > > cleanest way > > to go. > > I agree with Kees, set = {} and drop all the "something = 0;" s

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-04 Thread Christophe Leroy
Le 02/03/2024 à 02:51, Kees Cook a écrit : > On Sat, Mar 02, 2024 at 12:47:08AM +, Edgecombe, Rick P wrote: >> On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: >>> I totally understand. If the "uninitialized" warnings were actually >>> reliable, I would agree. I look at it this way: >>> >>

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-01 Thread Kees Cook
On Sat, Mar 02, 2024 at 12:47:08AM +, Edgecombe, Rick P wrote: > On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > > I totally understand. If the "uninitialized" warnings were actually > > reliable, I would agree. I look at it this way: > > > > - initializations can be missed either in sta

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-01 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > I totally understand. If the "uninitialized" warnings were actually > reliable, I would agree. I look at it this way: > > - initializations can be missed either in static initializers or via >   run time initializers. (So the risk of mistake he

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Christophe Leroy
Le 28/02/2024 à 18:01, Edgecombe, Rick P a écrit : > On Wed, 2024-02-28 at 13:22 +, Christophe Leroy wrote: >>> Any preference? Or maybe am I missing your point and talking >>> nonsense? >>> >> >> So my preference would go to the addition of: >> >> info.new_field = 0; >> >> But that'

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Kees Cook
On Wed, Feb 28, 2024 at 01:22:09PM +, Christophe Leroy wrote: > [...] > My worry with initialisation at declaration is it often hides missing > assignments. Let's take following simple exemple: > > char *colour(int num) > { > char *name; > > if (num == 0) { > name =

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 13:22 +, Christophe Leroy wrote: > > Any preference? Or maybe am I missing your point and talking > > nonsense? > > > > So my preference would go to the addition of: > > info.new_field = 0; > > But that's very minor and if you think it is easier to manage and

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Christophe Leroy
Le 27/02/2024 à 21:25, Edgecombe, Rick P a écrit : > On Tue, 2024-02-27 at 18:16 +, Christophe Leroy wrote: Why doing a full init of the struct when all fields are re- written a few lines after ? >>> >>> It's a nice change for robustness and makes future changes easier. >>> It'

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Kirill A. Shutemov
On Mon, Feb 26, 2024 at 11:09:47AM -0800, Rick Edgecombe wrote: > diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c > index 5db88b627439..dd6801bb9240 100644 > --- a/arch/alpha/kernel/osf_sys.c > +++ b/arch/alpha/kernel/osf_sys.c > @@ -1218,7 +1218,7 @@ static unsigned long >

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 18:16 +, Christophe Leroy wrote: > > > Why doing a full init of the struct when all fields are re- > > > written a few > > > lines after ? > > > > It's a nice change for robustness and makes future changes easier. > > It's > > not actually wasteful since the compiler will

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Christophe Leroy
Le 27/02/2024 à 19:07, Kees Cook a écrit : > On Tue, Feb 27, 2024 at 07:02:59AM +, Christophe Leroy wrote: >> >> >> Le 26/02/2024 à 20:09, Rick Edgecombe a écrit : >>> Future changes will need to add a field to struct vm_unmapped_area_info. >>> This would cause trouble for any archs that don'

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Kees Cook
On Tue, Feb 27, 2024 at 07:02:59AM +, Christophe Leroy wrote: > > > Le 26/02/2024 à 20:09, Rick Edgecombe a écrit : > > Future changes will need to add a field to struct vm_unmapped_area_info. > > This would cause trouble for any archs that don't initialize the > > struct. Currently every use

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 07:02 +, Christophe Leroy wrote: > > It could be possible to initialize the new field for each arch to > > 0, but > > instead simply inialize the field with a C99 struct inializing > > syntax. > > Why doing a full init of the struct when all fields are re-written a > few

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-26 Thread Christophe Leroy
Le 26/02/2024 à 20:09, Rick Edgecombe a écrit : > Future changes will need to add a field to struct vm_unmapped_area_info. > This would cause trouble for any archs that don't initialize the > struct. Currently every user sets each field, so if new fields are > added, the core code parsing the str