Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Greg Ungerer]

Hi Geert,

On 20/07/16 16:54, Geert Uytterhoeven wrote:
> Hi Nicolas,
> 
> On Wed, Jul 20, 2016 at 6:09 AM, Nicolas Pitre  
> wrote:
>> On Tue, 19 Jul 2016, Geert Uytterhoeven wrote:
>>
>>> On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
 Seeing as you have modified quite a few printk calls is it worth
 while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
>>>
>>> You mean pr_err(), pr_info(), ... ;-)
>>
>> Done.  Included in v3.
> 
> Really? The only change in v3 is the Reviewed-by?

Patch 2 in the series now does this:

[PATCH v3 02/12] binfmt_flat: convert printk invocations to their modern form

Regards
Greg


> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Greg Ungerer]

Hi Geert,

On 20/07/16 16:54, Geert Uytterhoeven wrote:
> Hi Nicolas,
> 
> On Wed, Jul 20, 2016 at 6:09 AM, Nicolas Pitre  
> wrote:
>> On Tue, 19 Jul 2016, Geert Uytterhoeven wrote:
>>
>>> On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
 Seeing as you have modified quite a few printk calls is it worth
 while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
>>>
>>> You mean pr_err(), pr_info(), ... ;-)
>>
>> Done.  Included in v3.
> 
> Really? The only change in v3 is the Reviewed-by?

Patch 2 in the series now does this:

[PATCH v3 02/12] binfmt_flat: convert printk invocations to their modern form

Regards
Greg


> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Geert Uytterhoeven]

Hi Nicolas,

On Wed, Jul 20, 2016 at 6:09 AM, Nicolas Pitre  wrote:
> On Tue, 19 Jul 2016, Geert Uytterhoeven wrote:
>
>> On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
>> > Seeing as you have modified quite a few printk calls is it worth
>> > while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
>>
>> You mean pr_err(), pr_info(), ... ;-)
>
> Done.  Included in v3.

Really? The only change in v3 is the Reviewed-by?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Geert Uytterhoeven]

Hi Nicolas,

On Wed, Jul 20, 2016 at 6:09 AM, Nicolas Pitre  wrote:
> On Tue, 19 Jul 2016, Geert Uytterhoeven wrote:
>
>> On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
>> > Seeing as you have modified quite a few printk calls is it worth
>> > while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
>>
>> You mean pr_err(), pr_info(), ... ;-)
>
> Done.  Included in v3.

Really? The only change in v3 is the Reviewed-by?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Nicolas Pitre]

On Tue, 19 Jul 2016, Geert Uytterhoeven wrote:

> On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
> > Seeing as you have modified quite a few printk calls is it worth
> > while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
> 
> You mean pr_err(), pr_info(), ... ;-)

Done.  Included in v3.


Nicolas

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Nicolas Pitre]

On Tue, 19 Jul 2016, Geert Uytterhoeven wrote:

> On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
> > Seeing as you have modified quite a few printk calls is it worth
> > while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
> 
> You mean pr_err(), pr_info(), ... ;-)

Done.  Included in v3.


Nicolas

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Geert Uytterhoeven]

On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
> Seeing as you have modified quite a few printk calls is it worth
> while annotating them with appropriate KERN_ERR, KERN_INFO, etc?

You mean pr_err(), pr_info(), ... ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Geert Uytterhoeven]

On Tue, Jul 19, 2016 at 6:52 AM, Greg Ungerer  wrote:
> Seeing as you have modified quite a few printk calls is it worth
> while annotating them with appropriate KERN_ERR, KERN_INFO, etc?

You mean pr_err(), pr_info(), ... ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Greg Ungerer]

Hi Nicolas,

On 18/07/16 13:31, Nicolas Pitre wrote:
> Remove excessive casts, do some code grouping, etc.
> No functional changes.
> 
> Signed-off-by: Nicolas Pitre 
> ---
>  fs/binfmt_flat.c | 118 
> ++-
>  1 file changed, 56 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index caf9e39bb8..085059d879 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -80,7 +80,7 @@ struct lib_info {
>   unsigned long text_len; /* Length of text 
> segment */
>   unsigned long entry;/* Start address for 
> this module */
>   unsigned long build_date;   /* When this one was 
> compiled */
> - short loaded;   /* Has this library 
> been loaded? */
> + bool loaded;/* Has this library 
> been loaded? */
>   } lib_list[MAX_SHARED_LIBS];
>  };
>  
> @@ -107,7 +107,7 @@ static struct linux_binfmt flat_format = {
>  static int flat_core_dump(struct coredump_params *cprm)
>  {
>   printk("Process %s:%d received signr %d and should have core dumped\n",
> - current->comm, current->pid, (int) 
> cprm->siginfo->si_signo);
> + current->comm, current->pid, cprm->siginfo->si_signo);
>   return(1);
>  }
>  
> @@ -190,7 +190,7 @@ static int decompress_exec(
>   loff_t fpos;
>   int ret, retval;
>  
> - DBG_FLT("decompress_exec(offset=%x,buf=%x,len=%x)\n",(int)offset, 
> (int)dst, (int)len);
> + DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, 
> len);
>  
>   memset(, 0, sizeof(strm));
>   strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
> @@ -358,8 +358,8 @@ calc_reloc(unsigned long r, struct lib_info *p, int 
> curid, int internalp)
>   text_len = p->lib_list[id].text_len;
>  
>   if (!flat_reloc_valid(r, start_brk - start_data + text_len)) {
> - printk("BINFMT_FLAT: reloc outside program 0x%x (0 - 
> 0x%x/0x%x)",
> -(int) 
> r,(int)(start_brk-start_data+text_len),(int)text_len);
> + printk("BINFMT_FLAT: reloc outside program 0x%lx (0 - 
> 0x%lx/0x%lx)",
> +r, start_brk-start_data+text_len, text_len);

Seeing as you have modified quite a few printk calls is it worth
while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
Just a thought.

Regards
Greg

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Greg Ungerer]

Hi Nicolas,

On 18/07/16 13:31, Nicolas Pitre wrote:
> Remove excessive casts, do some code grouping, etc.
> No functional changes.
> 
> Signed-off-by: Nicolas Pitre 
> ---
>  fs/binfmt_flat.c | 118 
> ++-
>  1 file changed, 56 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index caf9e39bb8..085059d879 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -80,7 +80,7 @@ struct lib_info {
>   unsigned long text_len; /* Length of text 
> segment */
>   unsigned long entry;/* Start address for 
> this module */
>   unsigned long build_date;   /* When this one was 
> compiled */
> - short loaded;   /* Has this library 
> been loaded? */
> + bool loaded;/* Has this library 
> been loaded? */
>   } lib_list[MAX_SHARED_LIBS];
>  };
>  
> @@ -107,7 +107,7 @@ static struct linux_binfmt flat_format = {
>  static int flat_core_dump(struct coredump_params *cprm)
>  {
>   printk("Process %s:%d received signr %d and should have core dumped\n",
> - current->comm, current->pid, (int) 
> cprm->siginfo->si_signo);
> + current->comm, current->pid, cprm->siginfo->si_signo);
>   return(1);
>  }
>  
> @@ -190,7 +190,7 @@ static int decompress_exec(
>   loff_t fpos;
>   int ret, retval;
>  
> - DBG_FLT("decompress_exec(offset=%x,buf=%x,len=%x)\n",(int)offset, 
> (int)dst, (int)len);
> + DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, 
> len);
>  
>   memset(, 0, sizeof(strm));
>   strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
> @@ -358,8 +358,8 @@ calc_reloc(unsigned long r, struct lib_info *p, int 
> curid, int internalp)
>   text_len = p->lib_list[id].text_len;
>  
>   if (!flat_reloc_valid(r, start_brk - start_data + text_len)) {
> - printk("BINFMT_FLAT: reloc outside program 0x%x (0 - 
> 0x%x/0x%x)",
> -(int) 
> r,(int)(start_brk-start_data+text_len),(int)text_len);
> + printk("BINFMT_FLAT: reloc outside program 0x%lx (0 - 
> 0x%lx/0x%lx)",
> +r, start_brk-start_data+text_len, text_len);

Seeing as you have modified quite a few printk calls is it worth
while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
Just a thought.

Regards
Greg

[PATCH v2 01/10] binfmt_flat: assorted cleanups

[Nicolas Pitre]

Remove excessive casts, do some code grouping, etc.
No functional changes.

Signed-off-by: Nicolas Pitre 
---
 fs/binfmt_flat.c | 118 ++-
 1 file changed, 56 insertions(+), 62 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index caf9e39bb8..085059d879 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -80,7 +80,7 @@ struct lib_info {
unsigned long text_len; /* Length of text 
segment */
unsigned long entry;/* Start address for 
this module */
unsigned long build_date;   /* When this one was 
compiled */
-   short loaded;   /* Has this library 
been loaded? */
+   bool loaded;/* Has this library 
been loaded? */
} lib_list[MAX_SHARED_LIBS];
 };
 
@@ -107,7 +107,7 @@ static struct linux_binfmt flat_format = {
 static int flat_core_dump(struct coredump_params *cprm)
 {
printk("Process %s:%d received signr %d and should have core dumped\n",
-   current->comm, current->pid, (int) 
cprm->siginfo->si_signo);
+   current->comm, current->pid, cprm->siginfo->si_signo);
return(1);
 }
 
@@ -190,7 +190,7 @@ static int decompress_exec(
loff_t fpos;
int ret, retval;
 
-   DBG_FLT("decompress_exec(offset=%x,buf=%x,len=%x)\n",(int)offset, 
(int)dst, (int)len);
+   DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, 
len);
 
memset(, 0, sizeof(strm));
strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
@@ -358,8 +358,8 @@ calc_reloc(unsigned long r, struct lib_info *p, int curid, 
int internalp)
text_len = p->lib_list[id].text_len;
 
if (!flat_reloc_valid(r, start_brk - start_data + text_len)) {
-   printk("BINFMT_FLAT: reloc outside program 0x%x (0 - 
0x%x/0x%x)",
-  (int) 
r,(int)(start_brk-start_data+text_len),(int)text_len);
+   printk("BINFMT_FLAT: reloc outside program 0x%lx (0 - 
0x%lx/0x%lx)",
+  r, start_brk-start_data+text_len, text_len);
goto failed;
}
 
@@ -383,7 +383,7 @@ failed:
 static void old_reloc(unsigned long rl)
 {
 #ifdef DEBUG
-   char *segment[] = { "TEXT", "DATA", "BSS", "*UNKNOWN*" };
+   static const char *segment[] = { "TEXT", "DATA", "BSS", "*UNKNOWN*" };
 #endif
flat_v2_reloc_t r;
unsigned long *ptr;
@@ -397,8 +397,8 @@ static void old_reloc(unsigned long rl)
 
 #ifdef DEBUG
printk("Relocation of variable at DATASEG+%x "
-   "(address %p, currently %x) into segment %s\n",
-   r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
+   "(address %p, currently %lx) into segment %s\n",
+   r.reloc.offset, ptr, *ptr, segment[r.reloc.type]);
 #endif

switch (r.reloc.type) {
@@ -417,7 +417,7 @@ static void old_reloc(unsigned long rl)
}
 
 #ifdef DEBUG
-   printk("Relocation became %x\n", (int)*ptr);
+   printk("Relocation became %lx\n", *ptr);
 #endif
 }  
 
@@ -427,17 +427,15 @@ static int load_flat_file(struct linux_binprm * bprm,
struct lib_info *libinfo, int id, unsigned long *extra_stack)
 {
struct flat_hdr * hdr;
-   unsigned long textpos = 0, datapos = 0, result;
-   unsigned long realdatastart = 0;
-   unsigned long text_len, data_len, bss_len, stack_len, flags;
-   unsigned long full_data;
-   unsigned long len, memp = 0;
-   unsigned long memp_size, extra, rlim;
-   unsigned long *reloc = 0, *rp;
+   unsigned long textpos, datapos, realdatastart;
+   unsigned long text_len, data_len, bss_len, stack_len, full_data, flags;
+   unsigned long len, memp, memp_size, extra, rlim;
+   unsigned long *reloc, *rp;
struct inode *inode;
-   int i, rev, relocs = 0;
+   int i, rev, relocs;
loff_t fpos;
unsigned long start_code, end_code;
+   ssize_t result;
int ret;
 
hdr = ((struct flat_hdr *) bprm->buf);  /* exec-header */
@@ -481,8 +479,8 @@ static int load_flat_file(struct linux_binprm * bprm,

/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
-   printk("BINFMT_FLAT: shared libraries are not available before 
rev 0x%x\n",
-   (int) FLAT_VERSION);
+   printk("BINFMT_FLAT: shared libraries are not available before 
rev 0x%lx\n",
+   FLAT_VERSION);
ret = -ENOEXEC;
goto err;
}
@@ -517,11 +515,9 @@ static int load_flat_file(struct linux_binprm * bprm,
 
/* Flush all traces of the currently running executable */
if (id == 0) {
-   result = 

[PATCH v2 01/10] binfmt_flat: assorted cleanups

[Nicolas Pitre]

Remove excessive casts, do some code grouping, etc.
No functional changes.

Signed-off-by: Nicolas Pitre 
---
 fs/binfmt_flat.c | 118 ++-
 1 file changed, 56 insertions(+), 62 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index caf9e39bb8..085059d879 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -80,7 +80,7 @@ struct lib_info {
unsigned long text_len; /* Length of text 
segment */
unsigned long entry;/* Start address for 
this module */
unsigned long build_date;   /* When this one was 
compiled */
-   short loaded;   /* Has this library 
been loaded? */
+   bool loaded;/* Has this library 
been loaded? */
} lib_list[MAX_SHARED_LIBS];
 };
 
@@ -107,7 +107,7 @@ static struct linux_binfmt flat_format = {
 static int flat_core_dump(struct coredump_params *cprm)
 {
printk("Process %s:%d received signr %d and should have core dumped\n",
-   current->comm, current->pid, (int) 
cprm->siginfo->si_signo);
+   current->comm, current->pid, cprm->siginfo->si_signo);
return(1);
 }
 
@@ -190,7 +190,7 @@ static int decompress_exec(
loff_t fpos;
int ret, retval;
 
-   DBG_FLT("decompress_exec(offset=%x,buf=%x,len=%x)\n",(int)offset, 
(int)dst, (int)len);
+   DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, 
len);
 
memset(, 0, sizeof(strm));
strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
@@ -358,8 +358,8 @@ calc_reloc(unsigned long r, struct lib_info *p, int curid, 
int internalp)
text_len = p->lib_list[id].text_len;
 
if (!flat_reloc_valid(r, start_brk - start_data + text_len)) {
-   printk("BINFMT_FLAT: reloc outside program 0x%x (0 - 
0x%x/0x%x)",
-  (int) 
r,(int)(start_brk-start_data+text_len),(int)text_len);
+   printk("BINFMT_FLAT: reloc outside program 0x%lx (0 - 
0x%lx/0x%lx)",
+  r, start_brk-start_data+text_len, text_len);
goto failed;
}
 
@@ -383,7 +383,7 @@ failed:
 static void old_reloc(unsigned long rl)
 {
 #ifdef DEBUG
-   char *segment[] = { "TEXT", "DATA", "BSS", "*UNKNOWN*" };
+   static const char *segment[] = { "TEXT", "DATA", "BSS", "*UNKNOWN*" };
 #endif
flat_v2_reloc_t r;
unsigned long *ptr;
@@ -397,8 +397,8 @@ static void old_reloc(unsigned long rl)
 
 #ifdef DEBUG
printk("Relocation of variable at DATASEG+%x "
-   "(address %p, currently %x) into segment %s\n",
-   r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
+   "(address %p, currently %lx) into segment %s\n",
+   r.reloc.offset, ptr, *ptr, segment[r.reloc.type]);
 #endif

switch (r.reloc.type) {
@@ -417,7 +417,7 @@ static void old_reloc(unsigned long rl)
}
 
 #ifdef DEBUG
-   printk("Relocation became %x\n", (int)*ptr);
+   printk("Relocation became %lx\n", *ptr);
 #endif
 }  
 
@@ -427,17 +427,15 @@ static int load_flat_file(struct linux_binprm * bprm,
struct lib_info *libinfo, int id, unsigned long *extra_stack)
 {
struct flat_hdr * hdr;
-   unsigned long textpos = 0, datapos = 0, result;
-   unsigned long realdatastart = 0;
-   unsigned long text_len, data_len, bss_len, stack_len, flags;
-   unsigned long full_data;
-   unsigned long len, memp = 0;
-   unsigned long memp_size, extra, rlim;
-   unsigned long *reloc = 0, *rp;
+   unsigned long textpos, datapos, realdatastart;
+   unsigned long text_len, data_len, bss_len, stack_len, full_data, flags;
+   unsigned long len, memp, memp_size, extra, rlim;
+   unsigned long *reloc, *rp;
struct inode *inode;
-   int i, rev, relocs = 0;
+   int i, rev, relocs;
loff_t fpos;
unsigned long start_code, end_code;
+   ssize_t result;
int ret;
 
hdr = ((struct flat_hdr *) bprm->buf);  /* exec-header */
@@ -481,8 +479,8 @@ static int load_flat_file(struct linux_binprm * bprm,

/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
-   printk("BINFMT_FLAT: shared libraries are not available before 
rev 0x%x\n",
-   (int) FLAT_VERSION);
+   printk("BINFMT_FLAT: shared libraries are not available before 
rev 0x%lx\n",
+   FLAT_VERSION);
ret = -ENOEXEC;
goto err;
}
@@ -517,11 +515,9 @@ static int load_flat_file(struct linux_binprm * bprm,
 
/* Flush all traces of the currently running executable */
if (id == 0) {
-   result =