diff mbox series

[PULL,11/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

Message ID 20210423203959.78275-2-imp@bsdimp.com (mailing list archive)
State New, archived
Headers show
Series [PULL,01/24] bsd-user: whitespace changes | expand

Commit Message

Warner Losh April 23, 2021, 8:39 p.m. UTC
From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/elfload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Philippe Mathieu-Daudé April 23, 2021, 9:23 p.m. UTC | #1
On 4/23/21 10:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh <imp@bsdimp.com>
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/elfload.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 87154283ef..07a00ddbd5 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs,
>                ibcs2_interpreter = 1;
>              }
>  
> -#if 0
> +#ifdef notyet

Better describe in the cover letter "ignored checkpatch errors" and keep
this unmodified rather than trying to bypass them by dubious code style
IMO. The checkpatch.pl script is here to help us ;)

>              printf("Using ELF interpreter %s\n", path(elf_interpreter));
>  #endif
Warner Losh April 23, 2021, 9:38 p.m. UTC | #2
On Fri, Apr 23, 2021 at 3:23 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> On 4/23/21 10:39 PM, imp@bsdimp.com wrote:
> > From: Warner Losh <imp@bsdimp.com>
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/elfload.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> > index 87154283ef..07a00ddbd5 100644
> > --- a/bsd-user/elfload.c
> > +++ b/bsd-user/elfload.c
> > @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm *bprm,
> struct target_pt_regs *regs,
> >                ibcs2_interpreter = 1;
> >              }
> >
> > -#if 0
> > +#ifdef notyet
>
> Better describe in the cover letter "ignored checkpatch errors" and keep
> this unmodified rather than trying to bypass them by dubious code style
> IMO. The checkpatch.pl script is here to help us ;)
>

This one I honestly was unsure about. To be honest, it's fear that kept
me keeping this code....  Maybe it would be even better to just delete
this code entirely. I have a working final state to pull from, now that I
think about it to forumlate a reply, so maybe that would be even
better?

Warner


> >              printf("Using ELF interpreter %s\n", path(elf_interpreter));
> >  #endif
>
Philippe Mathieu-Daudé April 23, 2021, 10:08 p.m. UTC | #3
On 4/23/21 11:38 PM, Warner Losh wrote:
> On Fri, Apr 23, 2021 at 3:23 PM Philippe Mathieu-Daudé <f4bug@amsat.org
> <mailto:f4bug@amsat.org>> wrote:
> 
>     On 4/23/21 10:39 PM, imp@bsdimp.com <mailto:imp@bsdimp.com> wrote:
>     > From: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>     >
>     > Signed-off-by: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>     > ---
>     >  bsd-user/elfload.c | 4 ++--
>     >  1 file changed, 2 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>     > index 87154283ef..07a00ddbd5 100644
>     > --- a/bsd-user/elfload.c
>     > +++ b/bsd-user/elfload.c
>     > @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm
>     *bprm, struct target_pt_regs *regs,
>     >                ibcs2_interpreter = 1;
>     >              }
>     > 
>     > -#if 0
>     > +#ifdef notyet
> 
>     Better describe in the cover letter "ignored checkpatch errors" and keep
>     this unmodified rather than trying to bypass them by dubious code style
>     IMO. The checkpatch.pl <http://checkpatch.pl> script is here to help
>     us ;)
> 
> 
> This one I honestly was unsure about. To be honest, it's fear that kept
> me keeping this code....  Maybe it would be even better to just delete
> this code entirely. I have a working final state to pull from, now that I
> think about it to forumlate a reply, so maybe that would be even
> better?

Personally I find it simpler. We use git, so we have the history in
the repository, can look at it and restore it if needed. This code
is dead since years.

Let's see what others think about this.

Regards,

Phil.
Richard Henderson April 23, 2021, 11 p.m. UTC | #4
On 4/23/21 3:08 PM, Philippe Mathieu-Daudé wrote:
> On 4/23/21 11:38 PM, Warner Losh wrote:
>> On Fri, Apr 23, 2021 at 3:23 PM Philippe Mathieu-Daudé <f4bug@amsat.org
>> <mailto:f4bug@amsat.org>> wrote:
>>
>>      On 4/23/21 10:39 PM, imp@bsdimp.com <mailto:imp@bsdimp.com> wrote:
>>      > From: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>>      >
>>      > Signed-off-by: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>>      > ---
>>      >  bsd-user/elfload.c | 4 ++--
>>      >  1 file changed, 2 insertions(+), 2 deletions(-)
>>      >
>>      > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>>      > index 87154283ef..07a00ddbd5 100644
>>      > --- a/bsd-user/elfload.c
>>      > +++ b/bsd-user/elfload.c
>>      > @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm
>>      *bprm, struct target_pt_regs *regs,
>>      >                ibcs2_interpreter = 1;
>>      >              }
>>      >
>>      > -#if 0
>>      > +#ifdef notyet
>>
>>      Better describe in the cover letter "ignored checkpatch errors" and keep
>>      this unmodified rather than trying to bypass them by dubious code style
>>      IMO. The checkpatch.pl <http://checkpatch.pl> script is here to help
>>      us ;)
>>
>>
>> This one I honestly was unsure about. To be honest, it's fear that kept
>> me keeping this code....  Maybe it would be even better to just delete
>> this code entirely. I have a working final state to pull from, now that I
>> think about it to forumlate a reply, so maybe that would be even
>> better?
> 
> Personally I find it simpler. We use git, so we have the history in
> the repository, can look at it and restore it if needed. This code
> is dead since years.
> 
> Let's see what others think about this.

Definitely better to remove.  We can review the new code more easily that way.


r~
diff mbox series

Patch

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 87154283ef..07a00ddbd5 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -1270,7 +1270,7 @@  int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs,
               ibcs2_interpreter = 1;
             }
 
-#if 0
+#ifdef notyet
             printf("Using ELF interpreter %s\n", path(elf_interpreter));
 #endif
             if (retval >= 0) {
@@ -1529,7 +1529,7 @@  int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs,
 
     padzero(elf_bss, elf_brk);
 
-#if 0
+#ifdef notyet
     printf("(start_brk) %x\n" , info->start_brk);
     printf("(end_code) %x\n" , info->end_code);
     printf("(start_code) %x\n" , info->start_code);