summary refs log tree commit diff
Commit message (Collapse)AuthorAge
...
* main: Print \n upon EOF (CTRL-D) when run interactivelyGerrit Pape2018-11-19
| | | | | | | | | | | | | | | | | | | | | | Exiting dash via a ^D instead of with "exit" causes dash to forget to print a newline. sh-3.1$ sh sh-3.1$ ^D sh-3.1$ dash $ sh-3.1$ It is more neat and tidy to send a newline similarly to what bash does, so it doesn't make the next prompt of the parent shell look ugly. Suggested by jidanni. Signed-off-by: Gerrit Pape <pape@smarden.org> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> [reworded the patch description] Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Bug-Debian: http://bugs.debian.org/476422 Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Report I/O error on stdoutGerrit Pape2018-11-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | ENOSPC as a result of an echo builting failing gives no diagnostic. Just as other shells, dash sets $? to 1, but aside from terminating the script, this does not inform the user what the problem is: zsh: % echo foo > /dev/full echo: write error: no space left on device bash: $ echo foo > /dev/full bash: echo: write error: No space left on device dash: $ echo foo > /dev/full [nothing] Print an error to stderr like the other shells. Suggested by Roger Leigh. Signed-off-by: Gerrit Pape <pape@smarden.org> [reworded the patch description with information from the bug] Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Bug-Debian: http://bugs.debian.org/690473 Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* builtin: Default to mktemp, not tempfileAndrej Shadura2018-11-19
| | | | | | | | Don't use tempfile, as it currently runs tempnam(), which is insecure and fails under pseudo(1). Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* shell: update .gitignoreMartijn Dekker2018-11-19
| | | | | | Ignore .deps and .dirstamp in all directories. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* man: Problems in dash.1, sh.1, sh.distrib.1esr@thyrsus.com2018-08-29
| | | | | | | | | | | | | | | | | This is automatically generated email about markup problems in a man page for which you appear to be responsible. If you are not the right person or list, please tell me so I can correct my database. See http://catb.org/~esr/doclifter/bugs.html for details on how and why these patches were generated. Feel free to email me with any questions. Note: These patches do not change the modification date of any manual page. You may wish to do that by hand. I apologize if this message seems spammy or impersonal. The volume of markup bugs I am tracking is over five hundred - there is no real alternative to generating bugmail from a database and template. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Ensure result is escaped in cvtnumHerbert Xu2018-08-29
| | | | | | | | | | | The minus sign generated from arithmetic expansion is currently unquoted which causes anomalies when the result is used in where the quoting matters. This patch fixes it by explicitly calling memtodest on the result in cvtnum. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* memalloc: Avoid looping in growstacktoHerbert Xu2018-08-29
| | | | | | | | Currently growstackto will repeatedly call growstackblock until the requisite size is obtained. This is wasteful. This patch changes growstackblock to take a minimum size instead. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Always set localvar_stopHerbert Xu2018-08-29
| | | | | | | | | | The variable localvar_stop is set iff vlocal is true. gcc doesn't get this so we get a spurious warning. This patch fixes this by always calling pushlocalvars with vlocal and making it only actually do the push if vlocal is non-zero. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Do not reprocess data when expanding wordsHerbert Xu2018-08-29
| | | | | | | | | | | | | | | | | | | | | | | | Currently various paths will reprocess data when performing word expansion. For example, expari will skip backwards looking for the start of the arithmetic expansion, while evalvar will skip unexpanded words manually. This is cumbersome and error-prone. This patch fixes this by making word expansions proceed in a linear fashion. This means changing argstr and the various expansion functions such as expari and subevalvar to return the next character to be expanded. This is inspired by similar code from FreeBSD. However, we take things one step further and completely remove the manual word skipping in evalvar. This is accomplished by introducing a new EXP_DISCARD flag that tells argstr to only parse and not produce any actual expansions. Incidentally, argstr will now always NUL-terminate the expansion unless the EXP_WORD flag is set. This is because all but one caller of argstr wants the result to be NUL-termianted. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Fix skipping of command substitution when trimming in evalvarHerbert Xu2018-08-29
| | | | | | | | | When we are trimming an unset variable in evalvar, any embedded command substitution that should have been skipped are not. This can cause them to be evaluated later should there be other command substitutions in the same input word. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Merge syntax/quotes in memtodest with flagsHerbert Xu2018-08-29
| | | | | | | | | | | | | | | | The function arguments syntax and quotes are both derived from the expansion flags. As syntax is only used by memtodest we do not need to maintain it outside of the function at all. The only place that uses something other than BASESYNTAX or DQSYNTAX is exptilde. However in that case DQSYNTAX has exactly the same effect as SQSYNTAX. This patch merges these two arguments into a single flags. The macro QUOTES_KEEPNUL has been renamed to EXP_KEEPNUL in order to keep the namespace separate. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Use HOME in tilde expansion when it is emptyHerbert Xu2018-08-29
| | | | | | | | Currently if HOME is set to empty tilde expansion will fail, i.e., it will remain as a literal tilde. This patch changes it to return the empty string as required by POSIX. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* shell: Don't include config.h for native helpersPeter Korsgaard2018-08-29
| | | | | | | | | | | | | | | | | | | | | | | config.h contains settings for the cross compiler (most importantly 32/64bit versions of functions), so don't include it when calling the native compiler to build the helpers. Otherwise we get build errors like: /usr/bin/gcc -include ../config.h -DBSD=1 -DSHELL -DIFS_BROKEN -g -O2 -Wall -o mkinit mkinit.c In file included from /usr/include/sys/stat.h:107, from /usr/include/fcntl.h:38, from mkinit.c:50: /usr/include/bits/stat.h:117: error: redefinition of ‘struct stat’ In file included from /usr/include/fcntl.h:38, from mkinit.c:50: /usr/include/sys/stat.h:504: error: redefinition of ‘stat’ /usr/include/sys/stat.h:455: note: previous definition of ‘stat’ was here Signed-off-by: Peter Korsgaard <peter@korsgaard.com> [baruch: apply to Makefile.am; update Peter's email address] Signed-off-by: Baruch Siach <baruch@tkos.co.il> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* builtin: Use test_access from NetBSD when faccessat is unavailableHerbert Xu2018-05-28
| | | | | | | | This patch adds the test_access code from NetBSD when faccess is unavailable. The code has been modified so that root can always read/write any file. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Add vfork supportHerbert Xu2018-05-28
| | | | | | This patch adds basic vfork support for the case of a simple command. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Replace with listsetvar with mklocal/setvareqHerbert Xu2018-05-28
| | | | | | | | | | | | This patch replaces listsetvar with mklocal/setvareq. As we now determine special built-in status prior to variable assignment, we no longer have to do a second pass listsetvar. Instead we will call setvareq directly instead of mklocal when necessary. In order to do this mklocal can now take a flag in order to mark a variable for export. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Fail immediately with redirections errors for simple commandHerbert Xu2018-05-28
| | | | | | | | Previously, dash would continue to perform variable expansions even if a redirection error occured. This patch changes it so that it fails immediately. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Add assignment built-in support againHerbert Xu2018-05-28
| | | | | | | | | | | This patch adds assignment built-in support that used to exist in dash prior to 0.3.8-15. This is because it will soon be part of POSIX, and the semantics are now much better defined. Recognition is done at execution time, so even "command -- export" or "var=export; command $var" should work. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* exec: Never rehash regular built-insHerbert Xu2018-05-28
| | | | | | | As regular (including special) built-ins can never be overridden, we should never remove them from the hash table. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* exec: Stricter pathopt parsingHerbert Xu2018-05-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes the parsing of pathopt. First of all only %builtin and %func (with arbitrary suffixes) will be recognised. Any other pathopt will be treated as a normal directory. Furthermore, pathopt can now be specified before the directory, rather than after it. In fact, a future version may remove support for pathopt suffixes. Wherever the pathopt is placed, an optional % may be placed after it to terminate the pathopt. This is so that it is less likely that a genuine directory containing a % sign is parsed as a pathopt. Users of padvance outside of exec.c have also been modified: 1) cd(1) will always treat % characters as part of the path. 2) chkmail will continue to accept arbitrary pathopt. 3) find_dot_file will ignore the %builtin pathopt instead of trying to do a stat in the accompanying directory (which is usually the current directory). The patch also removes the clearcmdentry optimisation where we attempt to only partially flush the table where possible. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* builtin: Mark more regular built-insHerbert Xu2018-05-28
| | | | | | | | | | | | This patch marks the following built-ins as regular, meaning that they cannot be overriden using PATH search: hash pwd type ulimit Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* exec: Do not allocate stack string in padvanceHerbert Xu2018-05-28
| | | | | | | | | | | Many callers of padvance immediately free the allocated string so this patch moves the stalloc call to the caller. Instead of returning the allocated string, padvance now returns the length to allocate (this may be longer than the actual string length, even including the NUL). For the case where we would previously return NULL, we now return -1. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* memalloc: Add growstackto helperHerbert Xu2018-05-28
| | | | | | | This patch adds the growstackto helper which repeatedly calls growstackblock until the requested size is reached. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* jobs: Replace some uses of fmtstr with stpcpy/stpncpyHerbert Xu2018-05-28
| | | | | | | | | Some uses of fmtstr, particularly the ones without a format string, can be replaced with stpcpy or stpncpy. This patch does that so we don't have to introduce unnecessary format strings in order to silence compiler warnings. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* output: Fix fmtstr return valueHerbert Xu2018-05-28
| | | | | | | The function fmtstr is meant to return the actual length of output produced, rather than the untruncated length. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* var: Set IFS to fixed value at start timeHerbert Xu2018-05-28
| | | | | | | | | This patch forces the IFS variable to always be set to its default value, regardless of the environment. It also removes the long unused IFS_BROKEN code. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* parser: Save/restore here-documents in command substitutionHerbert Xu2018-05-28
| | | | | | | | | | | | This patch changes the parsing of here-documents within command substitution, both old style and new style. In particular, the original here-document list is saved upon the beginning of parsing command substitution and restored when exiting. This means that here-documents outside of command substitution can no longer be filled by text within it and vice-versa. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* jobs: Only clear gotsigchld when waiting for everythingHerbert Xu2018-05-28
| | | | | | | | | | | | | | | | | | | | | The gotsigchld flag is always cleared in dowait but not all callers of dowait will wait for everything. In particular, when jp is set we only wait until the set job isn't running anymore. This patch fixes this by only clearing gotsigchld if jp is unset. It also changes the waitcmd to actually set jp which corresponds to the behaviour of bash/ksh93/mksh. The only other caller of dowait that doesn't wait for everything is the jobless reaper. This is in fact redundant now that we wait after every simple command. This patch removes it. Finally as every caller of dowait needs to wait until either the given job is not running, or until all terminated jobs have been processed, this patch moves the loop into dowait itself. Fixes: 03876c0743a5 ("eval: Reap zombies after built-in...") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* mkinit: Split reset into exitreset and resetHerbert Xu2018-05-28
| | | | | | | | | | | | | Previously reset was called after exitshell. This was changed so that it was called before exitshell because certain state needed to be reset in order for the EXIT trap to work. However, this caused issues because certain other states (such as local variables) should not be reset. This patch fixes this by creating a new function exitreset that is called prior to exitshell and moving reset back to its original location. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* main: Only set savestatus in exitcmdHerbert Xu2018-05-28
| | | | | | | | | | Currently exitcmd sets exitstatus and then savestatus if the latter was previously set. In fact, as exitcmd always raises an exception and will either end up in the setjmp call in main() or exitshell(), where exitstatus is always replaced by savestatus if set, we only need to set savestatus. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* exec: Return 126 on most errors in shellexecHerbert Xu2018-05-28
| | | | | | | | | | | Currently when shellexec fails on most errors the shell will exit with exit status 2. This patch changes it to 126 in order to avoid ambiguities with the exit status from a successful exec. The errors that result in 127 has also been expanded to include ENOTDIR, ENAMETOOLONG and ELOOP. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* Release 0.5.10.2.Herbert Xu2018-05-17
|
* parser: Fix incorrect eating of backslash newlinesHerbert Xu2018-05-15
| | | | | | | | | | | | | | With the introduction of synstack->syntax, a number of references to the syntax variable was missed during the conversion. This causes backslash newlines to be incorrectly removed in single quote context. This patch also combines these calls into a new helper function pgetc_top. Fixes: ab1cecb40478 ("parser: Add syntax stack for recursive...") Reported-by: Leah Neukirchen <leah@vuxu.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* Release 0.5.10.1.Herbert Xu2018-05-10
|
* jobs - Do not block when waiting on SIGCHLDHerbert Xu2018-05-09
| | | | | | | | | | | | | | | Because of the nature of SIGCHLD, the process may have already been waited on and therefore we must be prepared for the case that wait may block. So ensure that it doesn't by using WNOHANG. Furthermore, multiple jobs may have exited when gotsigchld is set. Therefore we need to wait until there are no zombies left. Lastly, waitforjob needs to be called with interrupts off and the original patch broke that. Fixes: 03876c0743a5 ("eval: Reap zombies after built-in...") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* Release 0.5.10.Herbert Xu2018-05-03
|
* eval: Variable assignments on functions are no longer persistentHerbert Xu2018-04-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Dirk Fieldhouse <fieldhouse@gmx.net> wrote: > > In POSIX.1-2017 ("simultaneously IEEE Std 1003.1™-2017 and The Open > Group Technical Standard Base Specifications, Issue 7") > <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09>, > we read under '2.9.1 Simple Commands' > > "Variable assignments shall be performed as follows: > ... > - If the command name is a standard utility implemented as a function > (see XBD Utility), the effect of variable assignments shall be as if the > utility was not implemented as a function. > ... > - If the command name is a function that is not a standard utility > implemented as a function, variable assignments shall affect the current > execution environment during the execution of the function. It is > unspecified: > > * Whether or not the variable assignments persist after the > completion of the function > > * Whether or not the variables gain the export attribute during > the execution of the function > > * Whether or not export attributes gained as a result of the > variable assignments persist after the completion of the function (if > variable assignments persist after the completion of the function)" POSIX used to require the current dash behaviour. However, you're right that this is no longer the case. This patch will remove the persistence of the variable assignment. I have considered the exporting the variables during the function execution but have decided against it because: 1) It makes the code bigger. 2) dash has never done this in the past. 3) You cannot use this portably anyway. Reported-by: Dirk Fieldhouse <fieldhouse@gmx.net> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* parser: Fix parameter expansion inside inner double quotesHerbert Xu2018-04-19
| | | | | | | | | | | | | | | | | The parsing of parameter expansion inside inner double quotes breaks because we never look for ENDVAR while innerdq is true. echo "${x#"${x+''}"''} This patch fixes it by pushing the syntax stack if innerdq is true and we enter a new parameter expansion. This patch also fixes a corner case where a bad substitution error occurs within arithmetic expansion. Reported-by: Denys Vlasenko <vda.linux@googlemail.com> Fixes: ab1cecb40478 (" parser: Add syntax stack for recursive...") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* parser: Fix parsing of ${}Herbert Xu2018-04-19
| | | | | | | | | | | dash -c 'echo ${}' should print "Bad subtitution" but instead fails with "Syntax error: Missing '}'". This is caused by us reading an extra character beyond the right brace. This patch fixes it so that this construct only fails during expansion rather than during parsing. Fixes: 3df3edd13389 ("[PARSER] Report substition errors at...") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* man: correct typos, iff -> ifMartijn Dekker2018-04-19
| | | | | | | | | | | | | | | Op 27-03-18 om 20:23 schreef Larry Hynes: > Funny, I did wonder if it might be a contraction, but I did find > it odd that it's not mentioned or explained. I'll leave it be, if > you all are happy enough to keep it 'as is', or can resubmit if you > think it's warranted. I think the simple fact that it came up here is evidence that this is too jargony for a manual. Patch attached. - M. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Do not quote backslashes in unquoted parameter expansionHerbert Xu2018-04-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On Mon, Mar 26, 2018 at 07:25:20PM +0200, Martijn Dekker wrote: > Op 26-03-18 om 17:38 schreef Harald van Dijk: > > And not by dash 0.5.4. Like I wrote, dash 0.5.5 had some bugs that were > > fixed in 0.5.6, which mostly restored the behaviour to match <0.5.5. > > Ah, sorry. dash 0.5.4 and earlier don't compile on my system, so they > are not included in my conveniently accessible arsenal of test shells. > > > As for my patches, that was by accident and doesn't work reliably. When > > the shell sees no metacharacters, pathname expansion is bypassed, and > > backslash isn't considered a metacharacter. Which got me to my original > > example of /de\v: there are no metacharacters in there, so the shell > > doesn't look to see if it matches anything. Which seems highly > > desirable: the shell shouldn't need to hit the file system for words not > > containing metacharacters. The only way then to get consistent behaviour > > is if the backslash is taken as quoted, so I'm not tempted to argue for > > the behaviour you're hoping for, sorry. :) Here is a better example: a="/*/\nullx" b="/*/\null"; printf "%s\n" $a $b dash currently prints /*/\nullx /*/\null bash prints /*/\nullx /dev/null You may argue the bash behaviour is inconsistent but it actually makes sense. What happens is that quote removal only applies to the original token as seen by the shell. It is never applied to the result of parameter expansion. Now you may ask why on earth does the second line say "/dev/null" instead of "/dev/\null". Well that's because it is not the quote removal step that removed the backslash, but the pathname expansion. The fact that the /de\v does not become /dev even though it exists is just the result of the optimisation to avoid unnecessarily calling stat(2). I have checked POSIX and I don't see anything that forbids this behaviour. So going back to dash yes I think we should adopt the bash behaviour for pathname expansion and keep the existing case semantics. This patch does exactly that. Note that this patch does not work unless you have already applied https://patchwork.kernel.org/patch/10306507/ because otherwise the optimisation mentioned above does not get detected correctly and we will end up doing quote removal twice. This patch also updates expmeta to handle naked backslashes at the end of the pattern which is now possible. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* shell: Add subdir-objects to AM_INIT_AUTOMAKEJason Bowen2018-04-19
| | | | | | | | | | | | | | I've attached a patch which adds the subdir-objects option to AM_INIT_AUTOMAKE. For a while now when I've compiled dash I received a warning from automake that there are source files in a subdirectory but that the subdir-objects automake option was not supplied. I've just been adding it myself, but I finally got around to submitting a patch. The code still compiles for now (i'm using automake 1.15.1), but warning text is rarely nice to see and, if the warning text is to be believed, then the warning will eventually become an error. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Restore input files in evalcommandHerbert Xu2018-04-19
| | | | | | | | | When evalcommand invokes a command that modifies parsefile and then bails out without popping the file, we need to ensure the input file is restored so that the shell can continue to execute. Reported-by: Martijn Dekker <martijn@inlv.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* eval: Reap zombies after built-in commands and functionsHerbert Xu2018-04-19
| | | | | | | | | | | | | Currently dash does not reap dead children after built-in commands or functions. This means that if you construct a loop consisting of solely built-in commands and functions, then zombies can hang around indefinitely. This patch fixes this by reaping when necessary after each built-in command and function. Reported-by: Denys Vlasenko <vda.linux@googlemail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* redir: Fix typo in noclobber codeHerbert Xu2018-04-19
| | | | | | | | The noclobber code has a typo in it that causes it to fail. This patch fixes it. Reported-by: Denys Vlasenko <vda.linux@googlemail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Fix glibc glob(3) supportHerbert Xu2018-04-19
| | | | | | | | | | | It's been a while since we disabled glob(3) support by default. It appears to be working now, however, we have to change our code to detect the no-match case correctly. In particular, we need to test for GLOB_NOMAGIC | GLOB_NOCHECK instead of GLOB_MAGCHAR. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Fix buffer overflow in expandmetaHerbert Xu2018-04-02
| | | | | | | | | | | | The native version of expandmeta allocates a buffer that may be overrun for two reasons. First of all the size is 1 byte too small but this is normally hidden because the minimum size is rounded up to 2048 bytes. Secondly, if the directory level is deep enough, any buffer can be overrun. This patch fixes both problems by calling realloc when necessary. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* builtin: Move echo space/nl handling into print_escape_strHerbert Xu2018-04-02
| | | | | | | | | Currently echocmd uses print_escape_str to do everything apart from printing the spaces/newlines separating its arguments. This patch moves the actual printing into print_escape_str as well using the format parameter. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* builtin: Fix echo performance regressionHerbert Xu2018-04-02
| | | | | | | | | | | | | | | | | | The commit d6c0e1e2ffbf7913ab69d51cc794d48d41c8fcb1 ("[BUILTIN] Handle embedded NULs correctly in printf") caused a performance regression in the echo built-in because every echo call now goes through the printf %b slow path where the string is always printed twice to ensure the space padding is correct in the presence of NUL characters. In fact this regression applies to printf %b as well. This is easily fixed by making printf %b take the fast path when no precision/field width modifiers are present. This patch also changes the second strchurnul call to strspn which generates slightly better code. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
* expand: Fix ghost fields with unquoted $@/$*Herbert Xu2018-04-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Harald van Dijk <harald@gigawatt.nl> wrote: > On 22/03/2018 22:38, Martijn Dekker wrote: >> Op 22-03-18 om 20:28 schreef Harald van Dijk: >>> On 22/03/2018 03:40, Martijn Dekker wrote: >>>> This patch fixes the bug that, given no positional parameters, unquoted >>>> $@ and $* incorrectly generate one empty field (they should generate no >>>> fields). Apparently that was a side effect of the above. >>> >>> This seems weird though. If you want to remove the recording of empty >>> regions because they are pointless, then how does removing them fix a >>> bug? Doesn't this show that empty regions do have an effect? Perhaps >>> they're not supposed to have any effect, perhaps it's a specific >>> combination of empty regions and something else that triggers some bug, >>> and perhaps that combination can no longer occur with your patch. >> >> The latter is my guess, but I haven't had time to investigate it. > > Looking into it again: > > When IFS is set to an empty string, sepc is set to '\0' in varvalue(). > This then causes *quotedp to be set to true, meaning evalvar()'s quoted > variable is turned on. quoted is then passed to recordregion() as the > nulonly parameter. > > ifsp->nulonly has a bigger effect than merely selecting whether to use > $IFS or whether to only split on null bytes: in ifsbreakup(), nulonly > also causes string termination to be suppressed. That's correct: that > special treatment is required to preserve empty fields in "$@" > expansion. But it should *only* be used when $@ is quoted: ifsbreakup() > takes nulonly from the last IFS region, even if it's empty, so having an > additional zero-length region with nulonly enabled causes confusion. > > Passing quoted by value to varvalue() and not attempting to modify it > should therefore, and in my quick testing does, also work to fix the > original $@ bug. You're right. The proper fix to this is to ensure that nulonly is not set in varvalue for $*. It should only be set for $@ when it's inside double quotes. In fact there is another bug while we're playing with $@/$*. When IFS is set to a non-whitespace character such as :, $* outside quotes won't remove empty fields as it should. This patch fixes both problems. Reported-by: Martijn Dekker <martijn@inlv.org> Suggested-by: Harald van Dijk <harald@gigawatt.nl> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
generated by cgit-pink 1.4.1 (git 2.36.1) at 2024-11-16 08:54:01 +0000