Re: [PATCH] Avoid double argument evaluation in MIN and MAX macros
On Wed, Apr 02, 2014 at 12:07:19PM +0200, Denys Vlasenko wrote:
> On Mon, Mar 31, 2014 at 12:08 AM, Bartosz Golaszewski
> wrote:
> > Commit c3a27b0b fixes a double argument evaluation by modifying the
> > macro invocation. What about preventing it on macro definition level?
> >
> > Signed-off-by: Bartosz Golaszewski
> > ---
> > include/libbb.h| 22 +++---
> > util-linux/swaponoff.c | 5 ++---
> > 2 files changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/libbb.h b/include/libbb.h
> > index 1cbe2c8..db75641 100644
> > --- a/include/libbb.h
> > +++ b/include/libbb.h
> > @@ -265,13 +265,21 @@ struct BUG_off_t_size_is_misdetected {
> > #define SKIP ((int) 2)
> >
> > /* Macros for min/max. */
> > -#ifndef MIN
> > -#define MIN(a,b) (((a)<(b))?(a):(b))
> > -#endif
> > -
> > -#ifndef MAX
> > -#define MAX(a,b) (((a)>(b))?(a):(b))
> > -#endif
> > +#undef MIN
> > +#define MIN(a,b) \
> > + ({ \
> > + __typeof__(a) _a = (a); \
> > + __typeof__(b) _b = (b); \
> > + _a < _b ? _a : _b; \
> > + })
> > +
> > +#undef MAX
> > +#define MAX(a,b) \
> > + ({ \
> > + __typeof__(a) _a = (a); \
> > + __typeof__(b) _b = (b); \
> > + _a > _b ? _a : _b; \
> > + })
>
> typeof() is GCCism. I am wary of using it unless it's necessary.
In addition I think everybody's used to MIN/MAX being macros subject
to multiple evaluation...
Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: dc hitting a compiler bug, or undefined behavior
On Mon, Mar 31, 2014 at 02:17:33PM +0200, Denys Vlasenko wrote:
> On Sun, Mar 30, 2014 at 11:18 AM, Lauri Kasanen wrote:
> > Hi,
> >
> > I'm seeing busybox dc acting funny when compiled with some versions of
> > gcc. This is on busybox git. The x86 binary busybox_unstripped and
> > config are attached.
> >
> > gcc 4.2.2 - ok
> > gcc 4.7.2:
> > nc 10 1 add p
> > 2.738e+93
> >
> > So either bb is hitting a compiler bug,
>
> Looks like a compiler bug:
>
> d = strtod(argument, &end);
> if (end != argument && *end == '\0') {
> push(d);
>
> is compiled to this:
>
>0x08049490 <+27>:call 0x80488b0
>0x08049495 <+32>:mov-0x14(%ebp),%eax
>0x08049498 <+35>:pop%ecx
>0x08049499 <+36>:pop%ebx
>0x0804949a <+37>:cmp%esi,%eax
>0x0804949c <+39>:je 0x80494b1
>0x0804949e <+41>:cmpb $0x0,(%eax)
>0x080494a1 <+44>:jne0x80494b5
>0x080494a3 <+46>:push %eax
>0x080494a4 <+47>:push %eax
>0x080494a5 <+48>:fstpl (%esp) < HERE
>0x080494a8 <+51>:call 0x8048f10
>
> Note how push(double a) argument gets passed on stack.
> But push() starts with this:
>
> Dump of assembler code for function push:
> => 0x08048f10 <+0>:fldl (%edi)
>
> which reads argument from some bogus location instead
> of stack:
>
> (gdb) nexti <=== EXECUTE fldl
> 0x08048f12 in push ()
> (gdb) p $st0
> $1 = 2.0554264135011661055418432229752339e-314
>
> That's not 10!
> 10 exists on stack all right:
>
> (gdb) p *(double*)($esp+4)
> $5 = 10
>
> the program just doesn't fetch it correctly.
It really looks to me like some non-default optimization is going on
and misbehaving. I've never seen gcc emit this kind of 'bare' function
without prologue/epilogue (or in this case, the ugly prologue moved
inside the conditional just before the function call). Is the busybox
build system doing anything behind the scenes (or perhaps with #pragma
or hidden attribte tags?) to get gcc to adjust the calling convention
for some functions?
Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: vi Undo patch
On Wed, Apr 2, 2014 at 7:31 PM, Jody Bruchon wrote:
> Denys, I've thrown this together which fixes the [Modified] not displaying.
> It looks like last_file_modified may be removable from the program entirely
> (it doesn't seem to actually be USED for anything) but I didn't want to do
> that just yet.
It seems to be used here:
// reduce counting -- the total lines can't have
// changed if we haven't done any edits.
if (file_modified != last_file_modified) {
tot = cur + count_lines(dot, end - 1) - 1;
last_file_modified = file_modified;
}
> I confirmed that this shows [Modified] the instant one makes a change, and
> when the undo stack is emptied out (and thus all modifications removed) it
> stops showing [Modified].
I thought about it a bit more and I think it's ok as-is:
[Modified] will appear anyway when user goes to command mode.
Thus, for example, accidental quitting without saving is not possible.
Next issue:
Looks like undo depth is not bounded?
What if user repeatedly inserts and deletes text ad infinitum -
will we eventually oom?
Do we even *want* to fix it?
"Undo as long as you have RAM" has its appeal too ;)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
AW: Why isn't FEATURE_SH_STANDALONE considered in BB_EXECVP()?
> Von: Mason > Gesendet: Mittwoch, 2. April 2014 14:27 > > Dietmar Schindler wrote: [snip] > > Your message is a reply to an existing thread: > > References: > <1396123321-23646-1-git-send-email-drew_mose...@mentor.com> > > > This tricks threading email agents into considering your message > as part of the original thread. Please don't reply to existing > threads when creating new ones. Oh, I'm sorry about that, will mind it in future. -- Best regards, Dietmar Schindler manroland web systems GmbH -- Management Board: Eckhard Hoerner-Marass (Spokesman), Joern Gossé Registered Office: Augsburg -- Trade Register: AG Augsburg -- HRB-No.: 26816 -- VAT: DE281389840 Confidentiality note: This eMail and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient, you are hereby notified that any use or dissemination of this communication is strictly prohibited. If you have received this eMail in error, then please delete this eMail. ! Please consider your environmental responsibility before printing this eMail ! ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Why isn't FEATURE_SH_STANDALONE considered in BB_EXECVP()?
Dietmar Schindler wrote: [snip] Your message is a reply to an existing thread: References: <1396123321-23646-1-git-send-email-drew_mose...@mentor.com> This tricks threading email agents into considering your message as part of the original thread. Please don't reply to existing threads when creating new ones. -- Regards. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Avoid double argument evaluation in MIN and MAX macros
2014-04-02 13:52 GMT+02:00 Mason :
>
> Would it be possible to define MIN and MAX as inline functions?
>
> static inline int MIN(int a, int b) { return a < b ? a : b; }
Macros accept any comparable type, so you would have to have specific
functions for int, unsigned, double etc.
2014-04-02 12:07 GMT+02:00 Denys Vlasenko :
> typeof() is GCCism. I am wary of using it unless it's necessary.
# find | egrep "\.[ch]$" | xargs grep "typeof"
./coreutils/sleep.c:ts.tv_sec = MAXINT(typeof(ts.tv_sec))
Typeof() is already used here and there are many other GCCisms all
over the place anyway.
Best regards,
Bartosz Golaszewski
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Why isn't FEATURE_SH_STANDALONE considered in BB_EXECVP()?
On the one hand, in shell/ash.c we have:
---
static void
tryexec(IF_FEATURE_SH_STANDALONE(int applet_no,) char *cmd, char **argv, char
**envp)
{
#if ENABLE_FEATURE_SH_STANDALONE
if (applet_no >= 0) {
if (APPLET_IS_NOEXEC(applet_no)) {
clearenv();
while (*envp)
putenv(*envp++);
run_applet_no_and_exit(applet_no, argv);
}
/* re-exec ourselves with the new arguments */
execve(bb_busybox_exec_path, argv, envp);
/* If they called chroot or otherwise made the binary no longer
* executable, fall through */
}
#endif
[...]
}
---
The invocation of run_applet_no_and_exit() allows us to execute busybox NOEXEC
applets from ash even if the busybox binary is no longer accessible.
On the other hand, in libbb/execable.c we have:
---
#if ENABLE_FEATURE_PREFER_APPLETS
/* just like the real execvp, but try to launch an applet named 'file' first */
int FAST_FUNC BB_EXECVP(const char *file, char *const argv[])
{
if (find_applet_by_name(file) >= 0)
execvp(bb_busybox_exec_path, argv);
return execvp(file, argv);
}
#endif
---
Here, regardless of FEATURE_SH_STANDALONE only execvp() is called. This leads
to cases where applets are not executed, for (contrived) example:
0>sudo busybox chroot /var/tmp rm /makefile.swp
chroot: can't execute 'rm': No such file or directory
Why doesn't BB_EXECVP() take FEATURE_SH_STANDALONE into account and handle it
similar to tryexec()?
--
Best regards,
Dietmar Schindler
manroland web systems GmbH -- Management Board: Eckhard Hoerner-Marass
(Spokesman), Joern Gossé
Registered Office: Augsburg -- Trade Register: AG Augsburg -- HRB-No.: 26816 --
VAT: DE281389840
Confidentiality note:
This eMail and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed. If
you are not the intended recipient, you are hereby notified that any use or
dissemination of this communication is strictly prohibited. If you have
received this eMail in error, then please delete this eMail.
! Please consider your environmental responsibility before printing this eMail !
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Avoid double argument evaluation in MIN and MAX macros
Bartosz Golaszewski wrote:
> Commit c3a27b0b fixes a double argument evaluation by modifying the
> macro invocation. What about preventing it on macro definition level?
>
> Signed-off-by: Bartosz Golaszewski
> ---
> include/libbb.h| 22 +++---
> util-linux/swaponoff.c | 5 ++---
> 2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index 1cbe2c8..db75641 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -265,13 +265,21 @@ struct BUG_off_t_size_is_misdetected {
> #define SKIP ((int) 2)
>
> /* Macros for min/max. */
> -#ifndef MIN
> -#define MIN(a,b) (((a)<(b))?(a):(b))
> -#endif
> -
> -#ifndef MAX
> -#define MAX(a,b) (((a)>(b))?(a):(b))
> -#endif
> +#undef MIN
> +#define MIN(a,b) \
> + ({ \
> + __typeof__(a) _a = (a); \
> + __typeof__(b) _b = (b); \
> + _a < _b ? _a : _b; \
> + })
> +
> +#undef MAX
> +#define MAX(a,b) \
> + ({ \
> + __typeof__(a) _a = (a); \
> + __typeof__(b) _b = (b); \
> + _a > _b ? _a : _b; \
> + })
Would it be possible to define MIN and MAX as inline functions?
static inline int MIN(int a, int b) { return a < b ? a : b; }
--
Regards.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Complete undo support for vi with intermediate queuing
On Sun, Mar 23, 2014 at 9:30 PM, j...@jodybruchon.com
wrote:
> I have spent a lot of time working on it, and I finally have this patch for
> complete undo support for BusyBox 'vi' with intermediate queuing capability.
> This patch INCLUDES my previous "undo patch v3" and also fixes the following
> issues with that patch:
>
> * Cursor placement when undoing is now correct
> * Undoing a 'dd' of the only line in the file no longer inserts a stray
> newline
> * Undoing 'J' no longer randomly destroys the first char of the line
>
> The biggest change is the "undo queuing" feature. When the undo queue feature
> is
> compiled in, single-character operations such as typing and backspacing will
> be
> grouped together. This greatly reduces malloc() calls while typing and makes
> undo much more useful; instead of each 'u' keypress undoing one typed
> character,
> all of the characters typed up to the queue size limit will be undone in one
> shot.
>
> The function undo_queue_commit() is called on certain keypresses to further
> increase the usefulness of queuing. Changing between typing and backspacing or
> moving the cursor during typing will "commit" the queue to an undo object,
> thereby splitting them into logical groupings.
>
> The queue size is configurable, the queue can be disabled, and undo support
> can
> be disabled completely, which will compile BusyBox vi identically to how it is
> built now. I have tested compilation AND as many of the possible text
> manipulation functions as I could with all possible combinations of undo
> support
> compiled in. I've tried to find bugs in the code in as many ways as possible,
> and it seems to work correctly under everything I have thrown at it. I doubt
> that this work on the undo feature can be improved any further.
>
> Denys, would you mind trying this final patch out and applying it?
It looks good. I did some changes, please review attached patch.
I noticed one bug so far: when I "busybox vi new_file", press i, and
start typing,
the status line isn't switching to "[Modified]". It used to.
diff -d -urpN busybox.8/editors/vi.c busybox.9/editors/vi.c
--- busybox.8/editors/vi.c 2014-02-24 17:00:45.0 +0100
+++ busybox.9/editors/vi.c 2014-04-02 13:45:37.044470361 +0200
@@ -17,7 +17,6 @@
* it would be easier to change the mark when add/delete lines
* More intelligence in refresh()
* ":r !cmd" and "!cmd" to filter text through an external command
- * A true "undo" facility
* An "ex" line oriented mode- maybe using "cmdedit"
*/
@@ -136,6 +135,36 @@
//config: cursor position using "ESC [ 6 n" escape sequence, then read stdin.
//config:
//config: This is not clean but helps a lot on serial lines and such.
+//config:config FEATURE_VI_UNDO
+//config: bool "Support undo command 'u'"
+//config: default y
+//config: depends on VI
+//config: help
+//config: Support the 'u' command to undo insertion, deletion, and replacement
+//config: of text.
+//config:config FEATURE_VI_UNDO_QUEUE
+//config: bool "Enable undo operation queuing"
+//config: default y
+//config: depends on FEATURE_VI_UNDO
+//config: help
+//config: The vi undo functions can use an intermediate queue to greatly lower
+//config: malloc() calls and overhead. When the maximum size of this queue is
+//config: reached, the contents of the queue are committed to the undo stack.
+//config: This increases the size of the undo code and allows some undo
+//config: operations (especially un-typing/backspacing) to be far more useful.
+//config:config FEATURE_VI_UNDO_QUEUE_MAX
+//config: int "Maximum undo character queue size"
+//config: default 256
+//config: range 32 65536
+//config: depends on FEATURE_VI_UNDO_QUEUE
+//config: help
+//config: This option sets the number of bytes used at runtime for the queue.
+//config: Smaller values will create more undo objects and reduce the amount
+//config: of typed or backspaced characters that are grouped into one undo
+//config: operation; larger values increase the potential size of each undo
+//config: and will generally malloc() larger objects and less frequently.
+//config: Unless you want more (or less) frequent "undo points" while typing,
+//config: you should probably leave this unchanged.
//applet:IF_VI(APPLET(vi, BB_DIR_BIN, BB_SUID_DROP))
@@ -347,6 +376,42 @@ struct globals {
char get_input_line__buf[MAX_INPUT_LEN]; /* former static */
char scr_out_buf[MAX_SCR_COLS + MAX_TABSTOP * 2];
+#if ENABLE_FEATURE_VI_UNDO
+// undo_push() operations
+#define UNDO_INS 0
+#define UNDO_DEL 1
+#define UNDO_INS_CHAIN 2
+#define UNDO_DEL_CHAIN 3
+// UNDO_*_QUEUED must be equal to UNDO_xxx ORed with UNDO_QUEUED_FLAG
+#define UNDO_QUEUED_FLAG 4
+#define UNDO_INS_QUEUED 4
+#define UNDO_DEL_QUEUED 5
+#define UNDO_USE_SPOS 32
+#define UNDO_EMPTY 64
+// Pass-through flags for functions that can be undone
+#define NO_UNDO 0
+#define ALLOW_UNDO 1
+#define ALLOW_UNDO_CHAIN 2
+#if ENABLE_FEATURE_VI_UNDO_QUEUE
+#define A
Re: [PATCH] build system: Specify '-nostldlib' when linking to .o files.
On Sat, Mar 29, 2014 at 9:02 PM, Drew Moseley wrote: > For certain cross build scenarios the LD variable is overridden > to use the gcc executive to ensure all the target tuning parameters > are used. In these cases, the executive errors out as shown below > but since this step is only linking to a .o file the standard libs > are not needed. > > $ make LD=gcc applets/built-in.o > LD applets/built-in.o > /usr/bin/ld: cannot find -lgcc_s > /usr/bin/ld: cannot find -lgcc_s > collect2: ld returned 1 exit status > make[1]: *** [applets/built-in.o] Error 1 > make: *** [applets_dir] Error 2 Applied, thanks! ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Avoid double argument evaluation in MIN and MAX macros
On Mon, Mar 31, 2014 at 12:08 AM, Bartosz Golaszewski
wrote:
> Commit c3a27b0b fixes a double argument evaluation by modifying the
> macro invocation. What about preventing it on macro definition level?
>
> Signed-off-by: Bartosz Golaszewski
> ---
> include/libbb.h| 22 +++---
> util-linux/swaponoff.c | 5 ++---
> 2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index 1cbe2c8..db75641 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -265,13 +265,21 @@ struct BUG_off_t_size_is_misdetected {
> #define SKIP ((int) 2)
>
> /* Macros for min/max. */
> -#ifndef MIN
> -#define MIN(a,b) (((a)<(b))?(a):(b))
> -#endif
> -
> -#ifndef MAX
> -#define MAX(a,b) (((a)>(b))?(a):(b))
> -#endif
> +#undef MIN
> +#define MIN(a,b) \
> + ({ \
> + __typeof__(a) _a = (a); \
> + __typeof__(b) _b = (b); \
> + _a < _b ? _a : _b; \
> + })
> +
> +#undef MAX
> +#define MAX(a,b) \
> + ({ \
> + __typeof__(a) _a = (a); \
> + __typeof__(b) _b = (b); \
> + _a > _b ? _a : _b; \
> + })
typeof() is GCCism. I am wary of using it unless it's necessary.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
