Hi Thorsten,

Thanks for your quick answer.

On jeu., 2017-02-09 at 18:46 +0000, Thorsten Glaser wrote:
> Jean Delvare dixit:
> >In mksh, "print -R" is not as close to "echo" as it was in original
> >ksh. Specifically, after "-R", mksh's print command keeps parsing
> >the command line in search of options, and stops as soon as it
> >finds an
> 
> Indeed, this is as documented:
> 
>     The -R option is used to emulate, to some degree, the BSD echo(1)
>     command which does not process ‘\’ sequences unless the -e option
>     is given.  As above, the -n option suppresses the trailing new‐
>     line.
> 
> The -R option does _not_ emulate the echo you mean but another.

I din't think -R currently emulates either properly (but I suppose I
can't complain, as "to some degree" could be read as implying just
that.) BSD echo will print arguments starting with a dash, other than a
leading -n. So if "print -R -42" is supposed to be like BSD "echo -42",
it should print "-42", as is the case with legacy ksh.

> We have the echo you mean in POSIX mode though…

Not sure what you mean here. I see that the echo command behavior is
changed if Flag(FPOSIX), but I can't seem to be able to set that flag
(I tried "export POSIX=1" but that doesn't seem to change anything?)

Also that would only change the behavior of echo anyway, not print.

> >Example with the original ksh:
> >
> >$ print -R -42
> >-42
> 
> Quick workaround (this will lose you support for -n though):
> 
> function print {
>         if [[ $1 = -R ]]; then
>                 shift
>                 builtin print -r -- "$@"
>         else
>                 builtin print "$@"
>         fi
> }

Actually I already suggested to the customer that they could replace
all the instances of "print -R" by "print -r --" in their scripts. They
did not answer yet.

I believe "print -r --" is a better choice in general regardless of the
outcome of our discussion, as -R will treat -n and -e (and combinations
thereof) as options, and the user may not expect that.

> >I looked at the mksh code and was able to modify the print_c function
> >to get mksh's print command to behave the same with -R as echo and the
> >original ksh print with -R. I have a patch ready. Are you interested in
> >it, or is the different behavior on purpose?
> 
> While I think you’re the first user of “print -R”, this looks as if it
> was intended — I’d address this on the customer side, maybe in a
> two-step process:
> 
> 1. Check all uses of 'print.*-[^ ]R' for which syntax is used, e.g.
>    if we need to handle “print -R -n”, “print -Rn”, “print -nR” or
>    somesuch; adjust the abovementioned print function, use it
> 
> 2. Convert all uses of print’s -R option to 'print -r --' or
>    'print -nr --' in the scripts (yes, more effort, but worth it).

I agree, and I would go that way if it was my code (straight to step 2,
even.) But it's not, and my experience is that customers are often
reluctant to change scripts that have been running for a long time and
are still deployed on many, sometimes heterogeneous, systems. I can
make suggestions but they get to make decisions.

> I’d advocate against trying to do something with echo that even
> pretends portability; we have no less than three different echo
> implementations in mksh alone (and which is chosen depends on
> several factors).

I did suggest to them to just use "echo" at one point and they turned
it down exactly because of portability concerns. Their script is
running on different OS flavors where echo behaves differently. They
are using "print -R" instead precisely because that appeared to be
portable across all their systems. Until they started migrating to
mksh, that is.

> Nevertheless, thank you for mailing about this!
> 
> 
> Hrm. Funnily enough:
> 
> tglase@tglase-nb:~ $ print -R -- -42
> -- -42
> tglase@tglase-nb:~ $ print -R -x -42
> /bin/mksh: print: -x: unknown option

I did notice this inconsistency too as part of my investigation, and
that's one of the reasons why I'm not sure the current behavior is
really by design.

> Perhaps I’ll have to investigate “the BSD echo(1) command”
> further (no problem thanks to TUHS) and maybe the implementation
> is indeed wrong… but the result will likely end up in a formal
> mksh release too late for your customer anyway. I’ll make a note.

That would not be a problem. If I know for sure that such a change will
make it into a future release of mksh, I can backport the change
immediately to whatever version our customers are running.

For reference, here is the patch I came up with. The idea is to jump to
the "echo" command handling code a soon as we see -R. The rest of the
changes is to remove po.pminusminus as it is no longer needed. Known
caveat: -E would be handled as a valid option, while it was not
supported by print -R before. Actually legacy ksh only supports -n
after -R (as BSD echo does), not -n nor -E.

Subject: print_c: Make option -R behave more like echo

In original ksh, print -R behaved like echo. Specifically it would
treat any unsupported option as if it was not an option, and simply
print it. Change the way we handle option -R to match this behavior.
---
 funcs.c |   27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

--- a/funcs.c
+++ b/funcs.c
@@ -284,6 +284,7 @@ c_print(const char **wp)
        const char *s;
        char *xp;
        XString xs;
+       bool new_exp, new_nl;
        struct {
                /* storage for columnisation */
                XPtrV words;
@@ -307,8 +308,6 @@ c_print(const char **wp)
                bool hist;
                /* print words as wide characters? */
                bool chars;
-               /* print a "--" argument? */
-               bool pminusminus;
                /* writing to a coprocess (SIGPIPE blocked)? */
                bool coproc;
                bool copipe;
@@ -348,7 +347,8 @@ c_print(const char **wp)
                        /* print everything as-is */
                        po.exp = false;
                } else {
-                       bool new_exp = po.exp, new_nl = po.nl;
+                       new_exp = po.exp;
+                       new_nl = po.nl;
 
                        /**
                         * a compromise between sysV and BSD echo commands:
@@ -387,8 +387,6 @@ c_print(const char **wp)
                const char *opts = "AclNnpRrsu,";
                const char *emsg;
 
-               po.pminusminus = false;
-
                while ((c = ksh_getopt(wp, &builtin_opt, opts)) != -1)
                        switch (c) {
                        case 'A':
@@ -418,10 +416,18 @@ c_print(const char **wp)
                                break;
                        case 'R':
                                /* fake BSD echo command */
-                               po.pminusminus = true;
-                               po.exp = false;
-                               opts = "en";
-                               break;
+                               new_exp = po.exp = false;
+                               new_nl = po.nl;
+                               if (wp[builtin_opt.optind - 1][builtin_opt.p] 
== '\0') {
+                                       /* R was the last option of its block */
+                                       wp += builtin_opt.optind;
+                                       goto print_tradparse_arg;
+                               } else {
+                                       /* R was not the last option of its 
block */
+                                       wp += builtin_opt.optind - 1;
+                                       s = *wp + builtin_opt.p;
+                                       goto print_tradparse_ch;
+                               }
                        case 'r':
                                po.exp = false;
                                break;
@@ -445,8 +451,7 @@ c_print(const char **wp)
                        if (wp[builtin_opt.optind] &&
                            ksh_isdash(wp[builtin_opt.optind]))
                                builtin_opt.optind++;
-               } else if (po.pminusminus)
-                       builtin_opt.optind--;
+               }
                wp += builtin_opt.optind;
        }
 

-- 
Jean Delvare
SUSE L3 Support

Reply via email to