disabled code in ksh tree.c

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

disabled code in ksh tree.c

Michael W. Bombardieri-2
Hello,

Revision 1.9 of tree.c (from 1999) added the disabled code and it
is still disabled. Would it be better to remove it?

- Michael


Index: tree.c
===================================================================
RCS file: /cvs/src/bin/ksh/tree.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 tree.c
--- tree.c 6 Jan 2018 16:28:58 -0000 1.30
+++ tree.c 11 Jan 2018 07:16:55 -0000
@@ -47,25 +47,8 @@ ptree(struct op *t, int indent, struct s
  fptreef(shf, indent, "#no-args# ");
  break;
  case TEXEC:
-#if 0 /* ?not useful - can't be called? */
- /* Print original vars */
- if (t->left->vars)
- for (w = t->left->vars; *w != NULL; )
- fptreef(shf, indent, "%S ", *w++);
- else
- fptreef(shf, indent, "#no-vars# ");
- /* Print expanded vars */
- if (t->args)
- for (w = t->args; *w != NULL; )
- fptreef(shf, indent, "%s ", *w++);
- else
- fptreef(shf, indent, "#no-args# ");
- /* Print original io */
- t = t->left;
-#else
  t = t->left;
  goto Chain;
-#endif
  case TPAREN:
  fptreef(shf, indent + 2, "( %T) ", t->left);
  break;

Reply | Threaded
Open this post in threaded view
|

Re: disabled code in ksh tree.c

Anton Lindqvist-2
On Thu, Jan 11, 2018 at 03:25:15PM +0800, Michael W. Bombardieri wrote:
> Hello,
>
> Revision 1.9 of tree.c (from 1999) added the disabled code and it
> is still disabled. Would it be better to remove it?

Fine with me. Anyone else willing OK?

> - Michael
>
>
> Index: tree.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/tree.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 tree.c
> --- tree.c 6 Jan 2018 16:28:58 -0000 1.30
> +++ tree.c 11 Jan 2018 07:16:55 -0000
> @@ -47,25 +47,8 @@ ptree(struct op *t, int indent, struct s
>   fptreef(shf, indent, "#no-args# ");
>   break;
>   case TEXEC:
> -#if 0 /* ?not useful - can't be called? */
> - /* Print original vars */
> - if (t->left->vars)
> - for (w = t->left->vars; *w != NULL; )
> - fptreef(shf, indent, "%S ", *w++);
> - else
> - fptreef(shf, indent, "#no-vars# ");
> - /* Print expanded vars */
> - if (t->args)
> - for (w = t->args; *w != NULL; )
> - fptreef(shf, indent, "%s ", *w++);
> - else
> - fptreef(shf, indent, "#no-args# ");
> - /* Print original io */
> - t = t->left;
> -#else
>   t = t->left;
>   goto Chain;
> -#endif
>   case TPAREN:
>   fptreef(shf, indent + 2, "( %T) ", t->left);
>   break;
>

Reply | Threaded
Open this post in threaded view
|

Re: disabled code in ksh tree.c

Jeremie Courreges-Anglas-2
On Sun, Jan 14 2018, Anton Lindqvist <[hidden email]> wrote:
> On Thu, Jan 11, 2018 at 03:25:15PM +0800, Michael W. Bombardieri wrote:
>> Hello,
>>
>> Revision 1.9 of tree.c (from 1999) added the disabled code and it
>> is still disabled. Would it be better to remove it?
>
> Fine with me. Anyone else willing OK?

I'd rather understand exactly what this code does and what the comment
actually means.  The TEXEC case can definitely be reached.

>> - Michael
>>
>>
>> Index: tree.c
>> ===================================================================
>> RCS file: /cvs/src/bin/ksh/tree.c,v
>> retrieving revision 1.30
>> diff -u -p -u -r1.30 tree.c
>> --- tree.c 6 Jan 2018 16:28:58 -0000 1.30
>> +++ tree.c 11 Jan 2018 07:16:55 -0000
>> @@ -47,25 +47,8 @@ ptree(struct op *t, int indent, struct s
>>   fptreef(shf, indent, "#no-args# ");
>>   break;
>>   case TEXEC:
>> -#if 0 /* ?not useful - can't be called? */
>> - /* Print original vars */
>> - if (t->left->vars)
>> - for (w = t->left->vars; *w != NULL; )
>> - fptreef(shf, indent, "%S ", *w++);
>> - else
>> - fptreef(shf, indent, "#no-vars# ");
>> - /* Print expanded vars */
>> - if (t->args)
>> - for (w = t->args; *w != NULL; )
>> - fptreef(shf, indent, "%s ", *w++);
>> - else
>> - fptreef(shf, indent, "#no-args# ");
>> - /* Print original io */
>> - t = t->left;
>> -#else
>>   t = t->left;
>>   goto Chain;
>> -#endif
>>   case TPAREN:
>>   fptreef(shf, indent + 2, "( %T) ", t->left);
>>   break;
>>
>

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: disabled code in ksh tree.c

Michael W. Bombardieri-2
On Sun, Jan 14, 2018 at 05:47:43PM +0100, Jeremie Courreges-Anglas wrote:

> On Sun, Jan 14 2018, Anton Lindqvist <[hidden email]> wrote:
> > On Thu, Jan 11, 2018 at 03:25:15PM +0800, Michael W. Bombardieri wrote:
> >> Hello,
> >>
> >> Revision 1.9 of tree.c (from 1999) added the disabled code and it
> >> is still disabled. Would it be better to remove it?
> >
> > Fine with me. Anyone else willing OK?
>
> I'd rather understand exactly what this code does and what the comment
> actually means.  The TEXEC case can definitely be reached.

The TEXEC case in ptree() happens if fptreef() or snptreef() are called with
format specifier %T.
From what I see it is called like this:

exchild() -> snptreef() -> vfptreef() -> ptree().

Here snptreef() is used to copy the command string to be executed into
a new Proc structure before exchild() forks and calls execute().

When calling ptree() by running shell command "ls -l", t->type is first TEXEC,
then after the goto t->type is TCOM.
So the switch statement appears to happen twice.

The comment "print original vars" does happen, but after the goto in TCOM case.
The if(t->vars) code in TCOM case is the same as the disabled if(t->left->vars)
code in TEXEC case.
Also, t->vars appears to be NULL in TEXEC case, but t->vars is set in TCOM case.
In terms of how the code is structured, vars belongs to TCOM not TEXEC.
So afaics the disabled code doesn't do anything useful.

- Michael

Reply | Threaded
Open this post in threaded view
|

Re: disabled code in ksh tree.c

Jeremie Courreges-Anglas-2
On Mon, Jan 15 2018, "Michael W. Bombardieri" <[hidden email]> wrote:

> On Sun, Jan 14, 2018 at 05:47:43PM +0100, Jeremie Courreges-Anglas wrote:
>> On Sun, Jan 14 2018, Anton Lindqvist <[hidden email]> wrote:
>> > On Thu, Jan 11, 2018 at 03:25:15PM +0800, Michael W. Bombardieri wrote:
>> >> Hello,
>> >>
>> >> Revision 1.9 of tree.c (from 1999) added the disabled code and it
>> >> is still disabled. Would it be better to remove it?
>> >
>> > Fine with me. Anyone else willing OK?
>>
>> I'd rather understand exactly what this code does and what the comment
>> actually means.  The TEXEC case can definitely be reached.
>
> The TEXEC case in ptree() happens if fptreef() or snptreef() are called with
> format specifier %T.
> From what I see it is called like this:
>
> exchild() -> snptreef() -> vfptreef() -> ptree().
>
> Here snptreef() is used to copy the command string to be executed into
> a new Proc structure before exchild() forks and calls execute().
>
> When calling ptree() by running shell command "ls -l", t->type is first TEXEC,
> then after the goto t->type is TCOM.
> So the switch statement appears to happen twice.
>
> The comment "print original vars" does happen, but after the goto in TCOM case.
> The if(t->vars) code in TCOM case is the same as the disabled if(t->left->vars)
> code in TEXEC case.
> Also, t->vars appears to be NULL in TEXEC case, but t->vars is set in TCOM case.
> In terms of how the code is structured, vars belongs to TCOM not TEXEC.
> So afaics the disabled code doesn't do anything useful.

Thanks for the explanation, ok jca@

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: disabled code in ksh tree.c

Anton Lindqvist-2
On Tue, Jan 16, 2018 at 04:29:59PM +0100, Jeremie Courreges-Anglas wrote:

> On Mon, Jan 15 2018, "Michael W. Bombardieri" <[hidden email]> wrote:
> > On Sun, Jan 14, 2018 at 05:47:43PM +0100, Jeremie Courreges-Anglas wrote:
> >> On Sun, Jan 14 2018, Anton Lindqvist <[hidden email]> wrote:
> >> > On Thu, Jan 11, 2018 at 03:25:15PM +0800, Michael W. Bombardieri wrote:
> >> >> Hello,
> >> >>
> >> >> Revision 1.9 of tree.c (from 1999) added the disabled code and it
> >> >> is still disabled. Would it be better to remove it?
> >> >
> >> > Fine with me. Anyone else willing OK?
> >>
> >> I'd rather understand exactly what this code does and what the comment
> >> actually means.  The TEXEC case can definitely be reached.
> >
> > The TEXEC case in ptree() happens if fptreef() or snptreef() are called with
> > format specifier %T.
> > From what I see it is called like this:
> >
> > exchild() -> snptreef() -> vfptreef() -> ptree().
> >
> > Here snptreef() is used to copy the command string to be executed into
> > a new Proc structure before exchild() forks and calls execute().
> >
> > When calling ptree() by running shell command "ls -l", t->type is first TEXEC,
> > then after the goto t->type is TCOM.
> > So the switch statement appears to happen twice.
> >
> > The comment "print original vars" does happen, but after the goto in TCOM case.
> > The if(t->vars) code in TCOM case is the same as the disabled if(t->left->vars)
> > code in TEXEC case.
> > Also, t->vars appears to be NULL in TEXEC case, but t->vars is set in TCOM case.
> > In terms of how the code is structured, vars belongs to TCOM not TEXEC.
> > So afaics the disabled code doesn't do anything useful.
>
> Thanks for the explanation, ok jca@

This has now been committed, thanks!