| Message ID | 1451930211-22460-2-git-send-email-mark.rutland@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Arnd, Sorry to poke during the merge window. Are you happy with the below patch, and if so, would you be happy for this to go via the arm64 tree? If possible, I'd like to be able to place the series on a stable branch come -rc1, so that can be used as the base for other work (e.g. [1]), and so that we can get it into -next. Everything else has the appropriate acks collected. Thanks, Mark. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/398527.html On Mon, Jan 04, 2016 at 05:56:34PM +0000, Mark Rutland wrote: > Currently __set_fixmap_offset is a macro function which has a local > variable called 'addr'. If a caller passes a 'phys' parameter which is > derived from a variable also called 'addr', the local variable will > shadow this, and the compiler will complain about the use of an > uninitialized variable. > > It is likely that fixmap users may use the name 'addr' for variables > that may be directly passed to __set_fixmap_offset, or that may be > indirectly generated via other macros. Rather than placing the burden on > callers to avoid the name 'addr', this patch changes __set_fixmap_offset > into a static inline function, avoiding namespace collisions. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Jeremy Linton <jeremy.linton@arm.com> > Cc: Laura Abbott <labbott@fedoraproject.org> > Cc: Will Deacon <will.deacon@arm.com> > --- > include/asm-generic/fixmap.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h > index 1cbb833..f9c27b6 100644 > --- a/include/asm-generic/fixmap.h > +++ b/include/asm-generic/fixmap.h > @@ -70,13 +70,13 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr) > #endif > > /* Return a pointer with offset calculated */ > -#define __set_fixmap_offset(idx, phys, flags) \ > -({ \ > - unsigned long addr; \ > - __set_fixmap(idx, phys, flags); \ > - addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \ > - addr; \ > -}) > +static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx, > + phys_addr_t phys, > + pgprot_t flags) > +{ > + __set_fixmap(idx, phys, flags); > + return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1)); > +} > > #define set_fixmap_offset(idx, phys) \ > __set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL) > -- > 1.9.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tuesday 19 January 2016 11:55:40 Mark Rutland wrote: > Hi Arnd, > > Sorry to poke during the merge window. > > Are you happy with the below patch, and if so, would you be happy for > this to go via the arm64 tree? > > If possible, I'd like to be able to place the series on a stable branch > come -rc1, so that can be used as the base for other work (e.g. [1]), > and so that we can get it into -next. > > Everything else has the appropriate acks collected. > > Yes, please merge it through arm64 Acked-by: Arnd Bergmann <arnd@arndb.de> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Jan 19, 2016 at 03:11:09PM +0100, Arnd Bergmann wrote: > On Tuesday 19 January 2016 11:55:40 Mark Rutland wrote: > > Hi Arnd, > > > > Sorry to poke during the merge window. > > > > Are you happy with the below patch, and if so, would you be happy for > > this to go via the arm64 tree? > > > > If possible, I'd like to be able to place the series on a stable branch > > come -rc1, so that can be used as the base for other work (e.g. [1]), > > and so that we can get it into -next. > > > > Everything else has the appropriate acks collected. > > > > > > Yes, please merge it through arm64 > > Acked-by: Arnd Bergmann <arnd@arndb.de> Cheers! Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jan 04, 2016 at 05:56:34PM +0000, Mark Rutland wrote: > Currently __set_fixmap_offset is a macro function which has a local > variable called 'addr'. If a caller passes a 'phys' parameter which is > derived from a variable also called 'addr', the local variable will > shadow this, and the compiler will complain about the use of an > uninitialized variable. > > It is likely that fixmap users may use the name 'addr' for variables > that may be directly passed to __set_fixmap_offset, or that may be > indirectly generated via other macros. Rather than placing the burden on > callers to avoid the name 'addr', this patch changes __set_fixmap_offset > into a static inline function, avoiding namespace collisions. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Jeremy Linton <jeremy.linton@arm.com> > Cc: Laura Abbott <labbott@fedoraproject.org> > Cc: Will Deacon <will.deacon@arm.com> > --- > include/asm-generic/fixmap.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) Acked-by: Will Deacon <will.deacon@arm.com> Catalin can pick this up for 4.6 along with Arnd's ack. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h index 1cbb833..f9c27b6 100644 --- a/include/asm-generic/fixmap.h +++ b/include/asm-generic/fixmap.h @@ -70,13 +70,13 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr) #endif /* Return a pointer with offset calculated */ -#define __set_fixmap_offset(idx, phys, flags) \ -({ \ - unsigned long addr; \ - __set_fixmap(idx, phys, flags); \ - addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \ - addr; \ -}) +static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx, + phys_addr_t phys, + pgprot_t flags) +{ + __set_fixmap(idx, phys, flags); + return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1)); +} #define set_fixmap_offset(idx, phys) \ __set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL)
Currently __set_fixmap_offset is a macro function which has a local variable called 'addr'. If a caller passes a 'phys' parameter which is derived from a variable also called 'addr', the local variable will shadow this, and the compiler will complain about the use of an uninitialized variable. It is likely that fixmap users may use the name 'addr' for variables that may be directly passed to __set_fixmap_offset, or that may be indirectly generated via other macros. Rather than placing the burden on callers to avoid the name 'addr', this patch changes __set_fixmap_offset into a static inline function, avoiding namespace collisions. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Jeremy Linton <jeremy.linton@arm.com> Cc: Laura Abbott <labbott@fedoraproject.org> Cc: Will Deacon <will.deacon@arm.com> --- include/asm-generic/fixmap.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel