Re: [PATCH v3 02/12] binfmt_flat: convert printk invocations to their modern form
On Tue, 19 Jul 2016, Joe Perches wrote:
> On Wed, 2016-07-20 at 00:20 -0400, Nicolas Pitre wrote:
> > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> []
> > @@ -15,6 +15,8 @@
> > * JAN/99 -- coded full program relocation (g...@snapgear.com)
> > */
> >
> > +#define pr_fmt(fmt)"BINFMT_FLAT: : " fmt
>
> Why the double colon?
Go figure.
> Much more common would be
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Sure. I used the all-caps version as that's what most former printk's
used. But if you say KBUILD_MODNAME is more common then I have no issue
with that.
>
> > @@ -106,8 +98,8 @@ 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, cprm->siginfo->si_signo);
> > + pr_warning("Process %s:%d received signr %d and should have core
> > dumped\n",
> > + current->comm, current->pid, cprm->siginfo->si_signo);
>
> Prefer pr_warn
OK.
Updated in my repo and pushed out.
> > return(1);
> > }
> >
> > @@ -190,17 +182,17 @@ static int decompress_exec(
> > loff_t fpos;
> > int ret, retval;
> >
> > - DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst,
> > len);
> > + pr_debug("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst,
> > len);
>
> Generally unnecessary as the function tracer works well
Not necessarily on uClinux where you might not aford it.
And this patch is about converting existing printk()'s so if some of
them should be removed then it would be best to do that separately.
> > memset(&strm, 0, sizeof(strm));
> > strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
> > if (strm.workspace == NULL) {
> > - DBG_FLT("binfmt_flat: no memory for decompress workspace\n");
> > + pr_debug("no memory for decompress workspace\n");
> > return -ENOMEM;
> > }
> > buf = kmalloc(LBUFSIZE, GFP_KERNEL);
> > if (buf == NULL) {
> > - DBG_FLT("binfmt_flat: no memory for read buffer\n");
> > + pr_debug("no memory for read buffer\n");
>
> Unnecessary OOM messages as allocs do a stack dump
Again this should probably be done separately.
Nicolas
Re: [PATCH v3 02/12] binfmt_flat: convert printk invocations to their modern form
On Wed, 2016-07-20 at 00:20 -0400, Nicolas Pitre wrote:
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
[]
> @@ -15,6 +15,8 @@
> * JAN/99 -- coded full program relocation (g...@snapgear.com)
> */
>
> +#define pr_fmt(fmt) "BINFMT_FLAT: : " fmt
Why the double colon?
Much more common would be
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -106,8 +98,8 @@ 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, cprm->siginfo->si_signo);
> + pr_warning("Process %s:%d received signr %d and should have core
> dumped\n",
> + current->comm, current->pid, cprm->siginfo->si_signo);
Prefer pr_warn
> return(1);
> }
>
> @@ -190,17 +182,17 @@ static int decompress_exec(
> loff_t fpos;
> int ret, retval;
>
> - DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst,
> len);
> + pr_debug("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst,
> len);
Generally unnecessary as the function tracer works well
>
> memset(&strm, 0, sizeof(strm));
> strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
> if (strm.workspace == NULL) {
> - DBG_FLT("binfmt_flat: no memory for decompress workspace\n");
> + pr_debug("no memory for decompress workspace\n");
> return -ENOMEM;
> }
> buf = kmalloc(LBUFSIZE, GFP_KERNEL);
> if (buf == NULL) {
> - DBG_FLT("binfmt_flat: no memory for read buffer\n");
> + pr_debug("no memory for read buffer\n");
Unnecessary OOM messages as allocs do a stack dump
[PATCH v3 02/12] binfmt_flat: convert printk invocations to their modern form
Signed-off-by: Nicolas Pitre
---
fs/binfmt_flat.c | 118 ---
1 file changed, 51 insertions(+), 67 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 085059d879..36f5bb6b2c 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -15,6 +15,8 @@
* JAN/99 -- coded full program relocation (g...@snapgear.com)
*/
+#define pr_fmt(fmt)"BINFMT_FLAT: : " fmt
+
#include
#include
#include
@@ -44,16 +46,6 @@
//
-#if 0
-#define DEBUG 1
-#endif
-
-#ifdef DEBUG
-#defineDBG_FLT(a...) printk(a)
-#else
-#defineDBG_FLT(a...)
-#endif
-
/*
* User data (data section and bss) needs to be aligned.
* We pick 0x20 here because it is the max value elf2flt has always
@@ -106,8 +98,8 @@ 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, cprm->siginfo->si_signo);
+ pr_warning("Process %s:%d received signr %d and should have core
dumped\n",
+ current->comm, current->pid, cprm->siginfo->si_signo);
return(1);
}
@@ -190,17 +182,17 @@ static int decompress_exec(
loff_t fpos;
int ret, retval;
- DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst,
len);
+ pr_debug("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst,
len);
memset(&strm, 0, sizeof(strm));
strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
if (strm.workspace == NULL) {
- DBG_FLT("binfmt_flat: no memory for decompress workspace\n");
+ pr_debug("no memory for decompress workspace\n");
return -ENOMEM;
}
buf = kmalloc(LBUFSIZE, GFP_KERNEL);
if (buf == NULL) {
- DBG_FLT("binfmt_flat: no memory for read buffer\n");
+ pr_debug("no memory for read buffer\n");
retval = -ENOMEM;
goto out_free;
}
@@ -218,25 +210,25 @@ static int decompress_exec(
/* Check minimum size -- gzip header */
if (ret < 10) {
- DBG_FLT("binfmt_flat: file too small?\n");
+ pr_debug("file too small?\n");
goto out_free_buf;
}
/* Check gzip magic number */
if ((buf[0] != 037) || ((buf[1] != 0213) && (buf[1] != 0236))) {
- DBG_FLT("binfmt_flat: unknown compression magic?\n");
+ pr_debug("unknown compression magic?\n");
goto out_free_buf;
}
/* Check gzip method */
if (buf[2] != 8) {
- DBG_FLT("binfmt_flat: unknown compression method?\n");
+ pr_debug("unknown compression method?\n");
goto out_free_buf;
}
/* Check gzip flags */
if ((buf[3] & ENCRYPTED) || (buf[3] & CONTINUATION) ||
(buf[3] & RESERVED)) {
- DBG_FLT("binfmt_flat: unknown flags?\n");
+ pr_debug("unknown flags?\n");
goto out_free_buf;
}
@@ -244,7 +236,7 @@ static int decompress_exec(
if (buf[3] & EXTRA_FIELD) {
ret += 2 + buf[10] + (buf[11] << 8);
if (unlikely(LBUFSIZE <= ret)) {
- DBG_FLT("binfmt_flat: buffer overflow (EXTRA)?\n");
+ pr_debug("buffer overflow (EXTRA)?\n");
goto out_free_buf;
}
}
@@ -252,7 +244,7 @@ static int decompress_exec(
while (ret < LBUFSIZE && buf[ret++] != 0)
;
if (unlikely(LBUFSIZE == ret)) {
- DBG_FLT("binfmt_flat: buffer overflow (ORIG_NAME)?\n");
+ pr_debug("buffer overflow (ORIG_NAME)?\n");
goto out_free_buf;
}
}
@@ -260,7 +252,7 @@ static int decompress_exec(
while (ret < LBUFSIZE && buf[ret++] != 0)
;
if (unlikely(LBUFSIZE == ret)) {
- DBG_FLT("binfmt_flat: buffer overflow (COMMENT)?\n");
+ pr_debug("buffer overflow (COMMENT)?\n");
goto out_free_buf;
}
}
@@ -273,7 +265,7 @@ static int decompress_exec(
strm.total_out = 0;
if (zlib_inflateInit2(&strm, -MAX_WBITS) != Z_OK) {
- DBG_FLT("binfmt_flat: zlib init failed?\n");
+ pr_debug("zlib init failed?\n");
goto out_free_buf;
}
@@ -290,7 +282,7 @@ static int decompress_exec(
}
if (ret < 0) {
- DBG_FLT("binfmt_flat: decompression failed (%d), %s\n",
+ pr_debug("decompression failed (%d), %s\n",
ret, st
