Re: svn commit: r336028 - head/usr.bin/top

2018-07-07 Thread Cy Schubert
In message , 
=?utf-8?B?5b6M6Je
k5aSn5Zyw?= writes:
> 
> > 2018/07/07 8:53、Hiroki Sato のメール:
> > 
> > Daichi GOTO  wrote
> >  in <201807061207.w66c76cr043...@repo.freebsd.org>:
> > 
> > da> Author: daichi
> > da> Date: Fri Jul  6 12:07:06 2018
> > da> New Revision: 336028
> > da> URL: https://svnweb.freebsd.org/changeset/base/336028
> > da>
> > da> Log:
> > da>   Changed to eliminate the upper limit of command length displayed
> > da>   by "-a" and expand to match terminal width
> > da>
> > da>   Reviewed by:  eadler
> > da>   Approved by:  gnn (mentor)
> > da>   Differential Revision:https://reviews.freebsd.org/D16083
> > da>
> > da> Modified:
> > da>   head/usr.bin/top/display.c
> > da>   head/usr.bin/top/machine.c
> > da>   head/usr.bin/top/screen.c
> > da>   head/usr.bin/top/top.h
> > 
> > This change breaks displaying a prompt and messages in the
> > interactive mode by new_message() when typing "o" or "p", for
> > example.  While r336031 fixed a warning in GCC, it does not fix the
> > problem itself.  Please fix it.
>
> OK. I will fix this problem first.

This should circumvent the problem until you find a more permanent fix.

Index: /opt/src/svn-current/usr.bin/top/display.c
===
--- /opt/src/svn-current/usr.bin/top/display.c  (revision 336075)
+++ /opt/src/svn-current/usr.bin/top/display.c  (working copy)
@@ -960,7 +960,7 @@
 va_start(args, msgfmt);
 
 /* first, format the message */
-vsnprintf(next_msg, strlen(next_msg), msgfmt, args);
+vsnprintf(next_msg, screen_width + 5, msgfmt, args);
 
 va_end(args);
 

>
>
> > I also think restructure of the buffer management is required first
> > if we want to eliminate the column width limitation.  Using sbuf(9)
> > consistently may be better than incomplete conversion from static
> > arrays to malloc().
>
> I understand. Switching to sbuf(9) is the next step.
>
> > 
> > -- Hiroki
>
>
>


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r336028 - head/usr.bin/top

2018-07-07 Thread 後藤大地 via svn-src-head

> 2018/07/07 8:53、Hiroki Sato のメール:
> 
> Daichi GOTO  wrote
>  in <201807061207.w66c76cr043...@repo.freebsd.org>:
> 
> da> Author: daichi
> da> Date: Fri Jul  6 12:07:06 2018
> da> New Revision: 336028
> da> URL: https://svnweb.freebsd.org/changeset/base/336028
> da>
> da> Log:
> da>   Changed to eliminate the upper limit of command length displayed
> da>   by "-a" and expand to match terminal width
> da>
> da>   Reviewed by:eadler
> da>   Approved by:gnn (mentor)
> da>   Differential Revision:  https://reviews.freebsd.org/D16083
> da>
> da> Modified:
> da>   head/usr.bin/top/display.c
> da>   head/usr.bin/top/machine.c
> da>   head/usr.bin/top/screen.c
> da>   head/usr.bin/top/top.h
> 
> This change breaks displaying a prompt and messages in the
> interactive mode by new_message() when typing "o" or "p", for
> example.  While r336031 fixed a warning in GCC, it does not fix the
> problem itself.  Please fix it.

OK. I will fix this problem first.


> I also think restructure of the buffer management is required first
> if we want to eliminate the column width limitation.  Using sbuf(9)
> consistently may be better than incomplete conversion from static
> arrays to malloc().

I understand. Switching to sbuf(9) is the next step.

> 
> -- Hiroki

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r336028 - head/usr.bin/top

2018-07-06 Thread Hiroki Sato
Daichi GOTO  wrote
  in <201807061207.w66c76cr043...@repo.freebsd.org>:

da> Author: daichi
da> Date: Fri Jul  6 12:07:06 2018
da> New Revision: 336028
da> URL: https://svnweb.freebsd.org/changeset/base/336028
da>
da> Log:
da>   Changed to eliminate the upper limit of command length displayed
da>   by "-a" and expand to match terminal width
da>
da>   Reviewed by:  eadler
da>   Approved by:  gnn (mentor)
da>   Differential Revision:https://reviews.freebsd.org/D16083
da>
da> Modified:
da>   head/usr.bin/top/display.c
da>   head/usr.bin/top/machine.c
da>   head/usr.bin/top/screen.c
da>   head/usr.bin/top/top.h

 This change breaks displaying a prompt and messages in the
 interactive mode by new_message() when typing "o" or "p", for
 example.  While r336031 fixed a warning in GCC, it does not fix the
 problem itself.  Please fix it.

 I also think restructure of the buffer management is required first
 if we want to eliminate the column width limitation.  Using sbuf(9)
 consistently may be better than incomplete conversion from static
 arrays to malloc().

-- Hiroki


pgpwYfF61gB5W.pgp
Description: PGP signature


Re: svn commit: r336028 - head/usr.bin/top

2018-07-06 Thread 後藤大地 via svn-src-head
Surely. I think your advice is appropriate.
Could you please commit?

> 2018/07/06 22:04、Sean Bruno のメール:
> 
> 
> 
> On 07/06/18 06:07, Daichi GOTO wrote:
>> -static char next_msg[MAX_COLS + 5];
>> +static char *next_msg = NULL;
>> static int msglen = 0;
>> /* Invariant: msglen is always the length of the message currently displayed
>>on the screen (even when next_msg doesn't contain that message). */
> 
> gcc noticed that a later call to vsnprintf() now has some problems.
> /home/sbruno/bsd/wifi/fbsd_head/usr.bin/top/display.c: In function
> 'new_message':
> /home/sbruno/bsd/wifi/fbsd_head/usr.bin/top/display.c:963:31: error:
> argument to 'sizeof' in 'vsnprintf' call is the same expression as the
> destination; did you mean to provide an explicit length?
> [-Werror=sizeof-pointer-memaccess]
> vsnprintf(next_msg, sizeof(next_msg), msgfmt, args);
> 
> 
> I think this needs to be changed
> 
> Index: usr.bin/top/display.c
> ===
> --- usr.bin/top/display.c   (revision 336029)
> +++ usr.bin/top/display.c   (working copy)
> @@ -960,7 +960,7 @@
> va_start(args, msgfmt);
> 
> /* first, format the message */
> -vsnprintf(next_msg, sizeof(next_msg), msgfmt, args);
> +vsnprintf(next_msg, strlen(next_msg), msgfmt, args);
> 
> va_end(args);
> 

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r336028 - head/usr.bin/top

2018-07-06 Thread Sean Bruno


On 07/06/18 06:07, Daichi GOTO wrote:
> -static char next_msg[MAX_COLS + 5];
> +static char *next_msg = NULL;
>  static int msglen = 0;
>  /* Invariant: msglen is always the length of the message currently displayed
> on the screen (even when next_msg doesn't contain that message). */

gcc noticed that a later call to vsnprintf() now has some problems.
/home/sbruno/bsd/wifi/fbsd_head/usr.bin/top/display.c: In function
'new_message':
/home/sbruno/bsd/wifi/fbsd_head/usr.bin/top/display.c:963:31: error:
argument to 'sizeof' in 'vsnprintf' call is the same expression as the
destination; did you mean to provide an explicit length?
[-Werror=sizeof-pointer-memaccess]
 vsnprintf(next_msg, sizeof(next_msg), msgfmt, args);


I think this needs to be changed

Index: usr.bin/top/display.c
===
--- usr.bin/top/display.c   (revision 336029)
+++ usr.bin/top/display.c   (working copy)
@@ -960,7 +960,7 @@
 va_start(args, msgfmt);

 /* first, format the message */
-vsnprintf(next_msg, sizeof(next_msg), msgfmt, args);
+vsnprintf(next_msg, strlen(next_msg), msgfmt, args);

 va_end(args);



signature.asc
Description: OpenPGP digital signature


svn commit: r336028 - head/usr.bin/top

2018-07-06 Thread Daichi GOTO
Author: daichi
Date: Fri Jul  6 12:07:06 2018
New Revision: 336028
URL: https://svnweb.freebsd.org/changeset/base/336028

Log:
  Changed to eliminate the upper limit of command length displayed
  by "-a" and expand to match terminal width
  
  Reviewed by:  eadler
  Approved by:  gnn (mentor)
  Differential Revision:https://reviews.freebsd.org/D16083

Modified:
  head/usr.bin/top/display.c
  head/usr.bin/top/machine.c
  head/usr.bin/top/screen.c
  head/usr.bin/top/top.h

Modified: head/usr.bin/top/display.c
==
--- head/usr.bin/top/display.c  Fri Jul  6 11:50:59 2018(r336027)
+++ head/usr.bin/top/display.c  Fri Jul  6 12:07:06 2018(r336028)
@@ -57,9 +57,8 @@ FILE *debug;
 static int lmpid = 0;
 static int last_hi = 0;/* used in u_process and u_endscreen */
 static int lastline = 0;
-static int display_width = MAX_COLS;
 
-#define lineindex(l) ((l)*display_width)
+#define lineindex(l) ((l)*screen_width)
 
 
 /* things initialized by display_init and used thruout */
@@ -94,6 +93,9 @@ static enum { OFF, ON, ERASE } header_status = ON;
 static void summary_format(char *, int *, const char * const *);
 static void line_update(char *, char *, int, int);
 
+static int setup_buffer_bufsiz = 0;
+static char * setup_buffer(char *, int);
+
 int  x_lastpid =   10;
 int  y_lastpid =   0;
 int  x_loadave =   33;
@@ -138,17 +140,9 @@ display_resize(void)
 
 if (lines < 0)
lines = 0;
-/* we don't want more than MAX_COLS columns, since the machine-dependent
-   modules make static allocations based on MAX_COLS and we don't want
-   to run off the end of their buffers */
-display_width = screen_width;
-if (display_width >= MAX_COLS)
-{
-   display_width = MAX_COLS - 1;
-}
 
 /* now, allocate space for the screen buffer */
-screenbuf = calloc(lines, display_width);
+screenbuf = calloc(lines, screen_width);
 if (screenbuf == NULL)
 {
/* oops! */
@@ -336,7 +330,7 @@ i_timeofday(time_t *tod)
 }
 
 static int ltotal = 0;
-static char procstates_buffer[MAX_COLS];
+static char *procstates_buffer = NULL;
 
 /*
  *  *_procstates(total, brkdn, names) - print the process summary line
@@ -350,6 +344,8 @@ i_procstates(int total, int *brkdn)
 {
 int i;
 
+procstates_buffer = setup_buffer(procstates_buffer, 0);
+
 /* write current number of processes and remember the value */
 printf("%d %s:", total, (ps.thread) ? "threads" :"processes");
 ltotal = total;
@@ -372,9 +368,11 @@ i_procstates(int total, int *brkdn)
 void
 u_procstates(int total, int *brkdn)
 {
-static char new[MAX_COLS];
+static char *new = NULL;
 int i;
 
+new = setup_buffer(new, 0);
+
 /* update number of processes only if it has changed */
 if (ltotal != total)
 {
@@ -551,11 +549,13 @@ z_cpustates(void)
  *for i_memory ONLY: cursor is on the previous line
  */
 
-static char memory_buffer[MAX_COLS];
+static char *memory_buffer = NULL;
 
 void
 i_memory(int *stats)
 {
+memory_buffer = setup_buffer(memory_buffer, 0);
+
 fputs("\nMem: ", stdout);
 lastline++;
 
@@ -567,8 +567,10 @@ i_memory(int *stats)
 void
 u_memory(int *stats)
 {
-static char new[MAX_COLS];
+static char *new = NULL;
 
+new = setup_buffer(new, 0);
+
 /* format the new line */
 summary_format(new, stats, memory_names);
 line_update(memory_buffer, new, x_mem, y_mem);
@@ -580,11 +582,13 @@ u_memory(int *stats)
  *  Assumptions:  cursor is on "lastline"
  *for i_arc ONLY: cursor is on the previous line
  */
-static char arc_buffer[MAX_COLS];
+static char *arc_buffer = NULL;
 
 void
 i_arc(int *stats)
 {
+arc_buffer = setup_buffer(arc_buffer, 0);
+
 if (arc_names == NULL)
return;
 
@@ -599,8 +603,10 @@ i_arc(int *stats)
 void
 u_arc(int *stats)
 {
-static char new[MAX_COLS];
+static char *new = NULL;
 
+new = setup_buffer(new, 0);
+
 if (arc_names == NULL)
return;
 
@@ -616,11 +622,13 @@ u_arc(int *stats)
  *  Assumptions:  cursor is on "lastline"
  *for i_carc ONLY: cursor is on the previous line
  */
-static char carc_buffer[MAX_COLS];
+static char *carc_buffer = NULL;
 
 void
 i_carc(int *stats)
 {
+carc_buffer = setup_buffer(carc_buffer, 0);
+
 if (carc_names == NULL)
return;
 
@@ -635,8 +643,10 @@ i_carc(int *stats)
 void
 u_carc(int *stats)
 {
-static char new[MAX_COLS];
+static char *new = NULL;
 
+new = setup_buffer(new, 0);
+
 if (carc_names == NULL)
return;
 
@@ -652,11 +662,13 @@ u_carc(int *stats)
  *for i_swap ONLY: cursor is on the previous line
  */
 
-static char swap_buffer[MAX_COLS];
+static char *swap_buffer = NULL;
 
 void
 i_swap(int *stats)
 {
+swap_buffer = setup_buffer(swap_buffer, 0);
+
 fputs("\nSwap: ", stdout);
 lastline++;
 
@@ -668,8 +680,10 @@ i_swap(