date:20200302

Re: /proc/partitions: add some space to avoid ragged output format

2020-03-02 Thread Thomas Wolff


Am 03.03.2020 um 00:26 schrieb Brian Inglis:

Hi Thomas,

In this industry, you plan ahead a bit further and longer: you need to go for at
least a three digit increase (below) or more; legacy 8TB HDDs are cheap and
common, 20TB are available, 100TB SSDs are available now (#blocks 9765625 -
11 digits), capacity is expanding *faster* than HDDs:

https://www.tomshardware.com/news/100tb-ssd-nimbus-sata-flash,36687.html

and speeds now exceed 6GB/s and 1M IO/s.

We're also at 64C/128T 6GHz (overclocked) chips with 256MB L3 and L4 caches,
256GB memory, and over the next decade, feature sizes dropping by an order of
magnitude from 14nm to 1.4nm, with corresponding increases, so maintainers
should consider capacity increases when they look at code.

To make this easier next time ;^> I'd define a couple of formats:

#define PROC_PARTITION_HDR  "%5s %5s %12s %s\n\n"
#define PROC_PARTITION_FMT  "%5d %5d %12U %s\n"

or simplify the code further with:

#define PROC_PARTITION_HDR  "%5s %5s %12s %-6s %-s\n\n"
#define PROC_PARTITION_FMT  "%5d %5d %12U %-6s %-s\n"

and sprintf the header into the buffer:

- print ("major minor  #blocks  name   win-mounts\n\n");
+ bufptr += __small_sprintf (bufptr, PROC_PARTITION_HDR,
+   "major", "minor", "#blocks",
+   "name   win-mounts\n\n");

*or*

- print ("major minor  #blocks  name   win-mounts\n\n");
+ bufptr += __small_sprintf (bufptr, PROC_PARTITION_HDR,
+   "major", "minor", "#blocks",
+   "name", "win-mounts\n\n");


Hi Brian,
yes, I thought about factoring out the format as well, but then only 
submitted a quick-and-dirty patch.

If you're suggesting a more solid solution, would you submit your patch?
Thomas



On 2020-03-02 14:39, Thomas Wolff wrote:

---
  winsup/cygwin/fhandler_proc.cc | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc
index 605a8443f..3373f3ef5 100644
--- a/winsup/cygwin/fhandler_proc.cc
+++ b/winsup/cygwin/fhandler_proc.cc
@@ -1491,7 +1491,7 @@ format_proc_partitions (void *, char *)
}
if (!got_one)
{
- print ("major minor  #blocks  name   win-mounts\n\n");
+ print ("major minor#blocks   name   win-mounts\n\n");
  got_one = true;
}
/* Fetch partition info for the entire disk to get its size. */
@@ -1514,7 +1514,7 @@ format_proc_partitions (void *, char *)
  size = 0;
}
device dev (drive_num, 0);
-  bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s\n",
+  bufptr += __small_sprintf (bufptr, "%5d %5d %12U %s\n",
 dev.get_major (), dev.get_minor (),
 size >> 10, dev.name () + 5);
/* Fetch drive layout info to get size of all partitions on the disk. */
@@ -1561,7 +1561,7 @@ format_proc_partitions (void *, char *)
  continue;
device dev (drive_num, part_num);

-   bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s",
+   bufptr += __small_sprintf (bufptr, "%5d %5d %12U %s",
   dev.get_major (), dev.get_minor (),
   size >> 10, dev.name () + 5);
/* Check if the partition is mounted in Windows and, if so,

Re: [PATCH 1/1] Collect handling of wpixput and wpbuf into a helper class.

2020-03-02 Thread Takashi Yano

Hi Hans,

Thanks for the patch.

On Tue, 3 Mar 2020 00:07:25 +0100
Hans-Bernhard Bröker wrote:
> +  inline void sendOut(HANDLE , DWORD *wn) { WriteConsoleA 
> (handle, buf, ixput, wn, 0); };

The second argument DWORD *wn of sendOut() is not used
outside sendOut(), so it can be covered up like:

inline void sendOut (HANDLE )
{
  DWORD wn;
  WriteConsoleA (handle, buf, ixput, , 0);
}

-- 
Takashi Yano

Re: /proc/partitions: add some space to avoid ragged output format

2020-03-02 Thread Brian Inglis

Hi Thomas,

In this industry, you plan ahead a bit further and longer: you need to go for at
least a three digit increase (below) or more; legacy 8TB HDDs are cheap and
common, 20TB are available, 100TB SSDs are available now (#blocks 9765625 -
11 digits), capacity is expanding *faster* than HDDs:

https://www.tomshardware.com/news/100tb-ssd-nimbus-sata-flash,36687.html

and speeds now exceed 6GB/s and 1M IO/s.

We're also at 64C/128T 6GHz (overclocked) chips with 256MB L3 and L4 caches,
256GB memory, and over the next decade, feature sizes dropping by an order of
magnitude from 14nm to 1.4nm, with corresponding increases, so maintainers
should consider capacity increases when they look at code.

To make this easier next time ;^> I'd define a couple of formats:

#define PROC_PARTITION_HDR  "%5s %5s %12s %s\n\n"
#define PROC_PARTITION_FMT  "%5d %5d %12U %s\n"

or simplify the code further with:

#define PROC_PARTITION_HDR  "%5s %5s %12s %-6s %-s\n\n"
#define PROC_PARTITION_FMT  "%5d %5d %12U %-6s %-s\n"

and sprintf the header into the buffer:

- print ("major minor  #blocks  name   win-mounts\n\n");
+ bufptr += __small_sprintf (bufptr, PROC_PARTITION_HDR,
+   "major", "minor", "#blocks",
+   "name   win-mounts\n\n");

*or*

- print ("major minor  #blocks  name   win-mounts\n\n");
+ bufptr += __small_sprintf (bufptr, PROC_PARTITION_HDR,
+   "major", "minor", "#blocks",
+   "name", "win-mounts\n\n");


On 2020-03-02 14:39, Thomas Wolff wrote:

---
 winsup/cygwin/fhandler_proc.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc
index 605a8443f..3373f3ef5 100644
--- a/winsup/cygwin/fhandler_proc.cc
+++ b/winsup/cygwin/fhandler_proc.cc
@@ -1491,7 +1491,7 @@ format_proc_partitions (void *, char *)
}
   if (!got_one)
{
- print ("major minor  #blocks  name   win-mounts\n\n");
+ print ("major minor#blocks   name   win-mounts\n\n");
  got_one = true;
}
   /* Fetch partition info for the entire disk to get its size. */
@@ -1514,7 +1514,7 @@ format_proc_partitions (void *, char *)
  size = 0;
}
   device dev (drive_num, 0);
-  bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s\n",
+  bufptr += __small_sprintf (bufptr, "%5d %5d %12U %s\n",
 dev.get_major (), dev.get_minor (),
 size >> 10, dev.name () + 5);
   /* Fetch drive layout info to get size of all partitions on the disk. */
@@ -1561,7 +1561,7 @@ format_proc_partitions (void *, char *)
  continue;
device dev (drive_num, part_num);

-   bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s",
+   bufptr += __small_sprintf (bufptr, "%5d %5d %12U %s",
   dev.get_major (), dev.get_minor (),
   size >> 10, dev.name () + 5);
/* Check if the partition is mounted in Windows and, if so,

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.

[PATCH 1/1] Collect handling of wpixput and wpbuf into a helper class.

2020-03-02 Thread Hans-Bernhard Bröker


Replace direct access to a pair of co-dependent variables
by calls to methods of a class that encapsulates their relation.

Also replace C #define by C++ class constant.
---
 winsup/cygwin/fhandler_console.cc | 135 --
 1 file changed, 70 insertions(+), 65 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc

index c5f269168..af2fb11a4 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -59,17 +59,22 @@ static struct fhandler_base::rabuf_t con_ra;
  /* Write pending buffer for ESC sequence handling
in xterm compatible mode */
-#define WPBUF_LEN 256
-static unsigned char wpbuf[WPBUF_LEN];
-static int wpixput;
 static unsigned char last_char;
 -static inline void
-wpbuf_put (unsigned char x)
+// simple helper class to accumulate output in a buffer
+// and send that to the console on request:
+static class WritePendingBuf
 {
-  if (wpixput < WPBUF_LEN)
-wpbuf[wpixput++] = x;
-}
+private:
+  static const size_t WPBUF_LEN = 256u;
+  unsigned char buf[WPBUF_LEN];
+  size_t ixput;
+
+public:
+  inline void put(unsigned char x) { if (ixput < WPBUF_LEN) { 
buf[ixput++] = x; } };

+  inline void empty() { ixput = 0u; };
+  inline void sendOut(HANDLE , DWORD *wn) { WriteConsoleA 
(handle, buf, ixput, wn, 0); };

+} wpbuf;
  static void
 beep ()
@@ -2030,10 +2035,10 @@ fhandler_console::char_command (char c)
  break;
 #endif
case 'b': /* REP */
- wpbuf_put (c);
+ wpbuf.put (c);
  if (wincap.has_con_esc_rep ())
/* Just send the sequence */
-   WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+   wpbuf.sendOut( get_output_handle (), );
  else if (last_char && last_char != '\n')
for (int i = 0; i < con.args[0]; i++)
  WriteConsoleA (get_output_handle (), _char, 1, , 0);
@@ -2041,9 +2046,9 @@ fhandler_console::char_command (char c)
case 'r': /* DECSTBM */
  con.scroll_region.Top = con.args[0] ? con.args[0] - 1 : 0;
  con.scroll_region.Bottom = con.args[1] ? con.args[1] - 1 : -1;
- wpbuf_put (c);
+ wpbuf.put (c);
  /* Just send the sequence */
- WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+ wpbuf.sendOut( get_output_handle (), );
  break;
case 'L': /* IL */
  if (wincap.has_con_broken_il_dl ())
@@ -2067,8 +2072,8 @@ fhandler_console::char_command (char c)
   y + 1 - con.b.srWindow.Top,
   srBottom + 1 - con.b.srWindow.Top);
  WriteConsoleA (get_output_handle (), buf, strlen (buf), , 0);
- wpbuf_put ('T');
- WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+ wpbuf.put ('T');
+ wpbuf.sendOut( get_output_handle (), );
  __small_sprintf (buf, "\033[%d;%dr",
   srTop + 1 - con.b.srWindow.Top,
   srBottom + 1 - con.b.srWindow.Top);
@@ -2079,9 +2084,9 @@ fhandler_console::char_command (char c)
}
  else
{
- wpbuf_put (c);
+ wpbuf.put (c);
  /* Just send the sequence */
- WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+ wpbuf.sendOut( get_output_handle (), );
}
  break;
case 'M': /* DL */
@@ -2095,8 +2100,8 @@ fhandler_console::char_command (char c)
   y + 1 - con.b.srWindow.Top,
   srBottom + 1 - con.b.srWindow.Top);
  WriteConsoleA (get_output_handle (), buf, strlen (buf), , 0);
- wpbuf_put ('S');
- WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+ wpbuf.put ('S');
+ wpbuf.sendOut( get_output_handle (), );
  __small_sprintf (buf, "\033[%d;%dr",
   srTop + 1 - con.b.srWindow.Top,
   srBottom + 1 - con.b.srWindow.Top);
@@ -2107,13 +2112,13 @@ fhandler_console::char_command (char c)
}
  else
{
- wpbuf_put (c);
+ wpbuf.put (c);
  /* Just send the sequence */
- WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+ wpbuf.sendOut( get_output_handle (), );
}
  break;
case 'J': /* ED */
- wpbuf_put (c);
+ wpbuf.put (c);
  if (con.args[0] == 3 && wincap.has_con_broken_csi3j ())
{ /* Workaround for broken CSI3J in Win10 1809 */
  CONSOLE_SCREEN_BUFFER_INFO sbi;
@@ -2131,7 +2136,7 @@ fhandler_console::char_command (char c)
}
  else
/* Just send the sequence */
-   WriteConsoleA (get_output_handle (), wpbuf, wpixput, , 0);
+   wpbuf.sendOut( get_output_handle (), );
  break;

[PATCH 0/1] Handle wpbuf in a more C++ style

2020-03-02 Thread Hans-Bernhard Bröker

Replace a relatively C-styled co-dependent pair of variables, a #define, 
and an inline function by a helper class containing their relation, 
because that is more in the C++ style.


Hans-Bernhard Broeker (1):
  Collect handling of wpixput and wpbuf into a helper class.

 winsup/cygwin/fhandler_console.cc | 135 --
 1 file changed, 70 insertions(+), 65 deletions(-)

--
2.21.0

Copyright License waiver

2020-03-02 Thread Hans-Bernhard Bröker


To whom it may concern,

I hereby grant anyone the rights to any use code I send here under the 
standard 2-clause BSD license statement as found, among other places, in 
winsup/cygwin/CONTRIBUTORS.


Hans-Bernhard Bröker

/proc/partitions: add some space to avoid ragged output format

2020-03-02 Thread Thomas Wolff



From 2e33b27e7d4683062f21a3082ec634a440ff9387 Mon Sep 17 00:00:00 2001
From: Thomas Wolff 
Date: Mon, 2 Mar 2020 22:36:56 +0100
Subject: [PATCH] avoid ragged output of /proc/partitions

---
 winsup/cygwin/fhandler_proc.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc
index 605a8443f..3373f3ef5 100644
--- a/winsup/cygwin/fhandler_proc.cc
+++ b/winsup/cygwin/fhandler_proc.cc
@@ -1491,7 +1491,7 @@ format_proc_partitions (void *, char *)
}
   if (!got_one)
{
- print ("major minor  #blocks  name   win-mounts\n\n");
+ print ("major minor   #blocks   name   win-mounts\n\n");
  got_one = true;
}
   /* Fetch partition info for the entire disk to get its size. */
@@ -1514,7 +1514,7 @@ format_proc_partitions (void *, char *)
  size = 0;
}
   device dev (drive_num, 0);
-  bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s\n",
+  bufptr += __small_sprintf (bufptr, "%5d %5d %11U %s\n",
 dev.get_major (), dev.get_minor (),
 size >> 10, dev.name () + 5);
   /* Fetch drive layout info to get size of all partitions on the disk. */
@@ -1561,7 +1561,7 @@ format_proc_partitions (void *, char *)
  continue;
device dev (drive_num, part_num);
 
-   bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s",
+   bufptr += __small_sprintf (bufptr, "%5d %5d %11U %s",
   dev.get_major (), dev.get_minor (),
   size >> 10, dev.name () + 5);
/* Check if the partition is mounted in Windows and, if so,
-- 
2.21.0

Re: [PATCH v2 1/4] Cygwin: console: Add workaround for broken IL/DL in xterm mode.

2020-03-02 Thread Jon Turney


On 02/03/2020 19:54, Corinna Vinschen wrote:

On Mar  2 18:03, Corinna Vinschen wrote:

On Mar  1 15:38, Takashi Yano wrote:

Hi Hans,

[...]


I pushed wpbuf_put as a simple inline function as a stop-gap
measure so Cygwin builds on gcc 9.2.0.


\o/

https://ci.appveyor.com/project/cygwin/cygwin/builds/31190427

Re: [PATCH v2 1/4] Cygwin: console: Add workaround for broken IL/DL in xterm mode.

2020-03-02 Thread Corinna Vinschen

On Mar  2 18:03, Corinna Vinschen wrote:
> On Mar  1 15:38, Takashi Yano wrote:
> > Hi Hans,
> > 
> > On Sat, 29 Feb 2020 19:10:02 +0100
> > Hans-Bernhard Bröker wrote:
> > > One more important note: the current implementation has a potential 
> > > buffer overrun issue, because it writes first, and only then checks 
> > > whether that may have overrun the buffer.  And the check itself is off 
> > > by one, too:
> > > 
> > > >wpbuf[wpixput++] = x; \
> > > >if (wpixput > WPBUF_LEN) \
> > > > wpixput--; \
> > > 
> > > That's why my latest code snippet does it differently:
> > > 
> > >  >  if (ixput < WPBUF_LEN)
> > >  >{
> > >  >  buf[ixput++] = x;
> > >  >}
> > 
> > Indeed. You are right. Thanks for pointing out that.
> > Another similar problem exists in console code of escape
> > sequence handling, so I will submit a patch for that.
> > 
> > As for wpbuf, please continue to fix.
> 
> Yeah, a patch in `git format-patch' format would be most welcome.
> 
> For a first-time contribution (and then never again) we also need
> a 2-clause BSD license waiver per https://cygwin.com/contrib.html

I pushed wpbuf_put as a simple inline function as a stop-gap
measure so Cygwin builds on gcc 9.2.0.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature

Re: [PATCH 0/4] Cygwin: console: Some fixes for console in xterm mode.

2020-03-02 Thread Corinna Vinschen

On Mar  2 10:12, Takashi Yano wrote:
> Takashi Yano (4):
>   Cygwin: console: Revise the code to fix tab position.
>   Cygwin: console: Fix setting/unsetting xterm mode for input.
>   Cygwin: console: Prevent buffer overrun.
>   Cygwin: console: Add a workaround for "ESC 7" and "ESC 8".
> 
>  winsup/cygwin/fhandler.h  |  1 +
>  winsup/cygwin/fhandler_console.cc | 91 ++-
>  2 files changed, 55 insertions(+), 37 deletions(-)
> 
> -- 
> 2.21.0

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature

Re: [PATCH v2 1/4] Cygwin: console: Add workaround for broken IL/DL in xterm mode.

2020-03-02 Thread Corinna Vinschen

On Mar  1 15:38, Takashi Yano wrote:
> Hi Hans,
> 
> On Sat, 29 Feb 2020 19:10:02 +0100
> Hans-Bernhard Bröker wrote:
> > One more important note: the current implementation has a potential 
> > buffer overrun issue, because it writes first, and only then checks 
> > whether that may have overrun the buffer.  And the check itself is off 
> > by one, too:
> > 
> > >wpbuf[wpixput++] = x; \
> > >if (wpixput > WPBUF_LEN) \
> > > wpixput--; \
> > 
> > That's why my latest code snippet does it differently:
> > 
> >  >  if (ixput < WPBUF_LEN)
> >  >{
> >  >  buf[ixput++] = x;
> >  >}
> 
> Indeed. You are right. Thanks for pointing out that.
> Another similar problem exists in console code of escape
> sequence handling, so I will submit a patch for that.
> 
> As for wpbuf, please continue to fix.

Yeah, a patch in `git format-patch' format would be most welcome.

For a first-time contribution (and then never again) we also need
a 2-clause BSD license waiver per https://cygwin.com/contrib.html


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature

Re: /proc/partitions: add some space to avoid ragged output format

Re: [PATCH 1/1] Collect handling of wpixput and wpbuf into a helper class.

Re: /proc/partitions: add some space to avoid ragged output format

[PATCH 1/1] Collect handling of wpixput and wpbuf into a helper class.

[PATCH 0/1] Handle wpbuf in a more C++ style

Copyright License waiver

/proc/partitions: add some space to avoid ragged output format

Re: [PATCH v2 1/4] Cygwin: console: Add workaround for broken IL/DL in xterm mode.

Re: [PATCH v2 1/4] Cygwin: console: Add workaround for broken IL/DL in xterm mode.

Re: [PATCH 0/4] Cygwin: console: Some fixes for console in xterm mode.

Re: [PATCH v2 1/4] Cygwin: console: Add workaround for broken IL/DL in xterm mode.

11 matches

Site Navigation

Mail list logo

Footer information