Created new API for priting tables, mainly to solve alignment problems.
Implemented these test to virsh list. In the future, API may be
everywhere in virsh and virt-admin.
Also wrote basic tests for the new API, and corrected tests in virshtest
which are influenced by implementation of the API in virsh list.

Changes in v5:
- cleanup and merged code for calculating zero-width, nonprintable and combined
character.
- replaced virBufferAddStr with virBufferAddChar in some places
- in tests moved code for setting correct locale
- fixed few leaks and unitialized values

Changes in v4:
- fixed width calculation for zero-width, nonprintable and combined
character. (pulled some code from linux-util)
- added tests for cases mentioned above
- changed usage of vshControl variables. From now on PrintToStdout calls
PrintToString and then prints returned string to stdout

Changes in v3:
- changed encoding of 3/3 patch, otherwise it cannot be applied

Changes in v2:
- added tests
- fixed alignment for unicode character which span more spaces
- moved ncolumns check to vshTableRowAppend
- changed arguments for functions vshTablePrint, vshTablePrintToStdout,
    vshTablePrintToString

Simon Kobyda (3):
  vsh: Add API for printing tables.
  virsh: Implement new table API for virsh list
  vsh: Added tests

 tests/Makefile.am            |   8 +
 tests/virshtest.c            |  14 +-
 tests/vshtabletest.c         | 377 +++++++++++++++++++++++++++++
 tools/Makefile.am            |   4 +-
 tools/virsh-domain-monitor.c |  43 ++--
 tools/vsh-table.c            | 449 +++++++++++++++++++++++++++++++++++
 tools/vsh-table.h            |  42 ++++
 7 files changed, 910 insertions(+), 27 deletions(-)
 create mode 100644 tests/vshtabletest.c
 create mode 100644 tools/vsh-table.c
 create mode 100644 tools/vsh-table.h

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 08/23/2018 05:53 PM, Simon Kobyda wrote:
> Created new API for priting tables, mainly to solve alignment problems.
> Implemented these test to virsh list. In the future, API may be
> everywhere in virsh and virt-admin.
> Also wrote basic tests for the new API, and corrected tests in virshtest
> which are influenced by implementation of the API in virsh list.
> 
> Changes in v5:
> - cleanup and merged code for calculating zero-width, nonprintable and combined
> character.
> - replaced virBufferAddStr with virBufferAddChar in some places
> - in tests moved code for setting correct locale
> - fixed few leaks and unitialized values
> 
> Changes in v4:
> - fixed width calculation for zero-width, nonprintable and combined
> character. (pulled some code from linux-util)
> - added tests for cases mentioned above
> - changed usage of vshControl variables. From now on PrintToStdout calls
> PrintToString and then prints returned string to stdout
> 
> Changes in v3:
> - changed encoding of 3/3 patch, otherwise it cannot be applied
> 
> Changes in v2:
> - added tests
> - fixed alignment for unicode character which span more spaces
> - moved ncolumns check to vshTableRowAppend
> - changed arguments for functions vshTablePrint, vshTablePrintToStdout,
>     vshTablePrintToString
> 
> Simon Kobyda (3):
>   vsh: Add API for printing tables.
>   virsh: Implement new table API for virsh list
>   vsh: Added tests
> 
>  tests/Makefile.am            |   8 +
>  tests/virshtest.c            |  14 +-
>  tests/vshtabletest.c         | 377 +++++++++++++++++++++++++++++
>  tools/Makefile.am            |   4 +-
>  tools/virsh-domain-monitor.c |  43 ++--
>  tools/vsh-table.c            | 449 +++++++++++++++++++++++++++++++++++
>  tools/vsh-table.h            |  42 ++++
>  7 files changed, 910 insertions(+), 27 deletions(-)
>  create mode 100644 tests/vshtabletest.c
>  create mode 100644 tools/vsh-table.c
>  create mode 100644 tools/vsh-table.h
> 

ACKed and pushed.

Now we can start converting the rest of the code to use vshTable APIs.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> On 08/23/2018 05:53 PM, Simon Kobyda wrote:
> > Created new API for priting tables, mainly to solve alignment problems.
> > Implemented these test to virsh list. In the future, API may be
> > everywhere in virsh and virt-admin.
> > Also wrote basic tests for the new API, and corrected tests in virshtest
> > which are influenced by implementation of the API in virsh list.
> > 
> > Changes in v5:
> > - cleanup and merged code for calculating zero-width, nonprintable and combined
> > character.
> > - replaced virBufferAddStr with virBufferAddChar in some places
> > - in tests moved code for setting correct locale
> > - fixed few leaks and unitialized values
> > 
> > Changes in v4:
> > - fixed width calculation for zero-width, nonprintable and combined
> > character. (pulled some code from linux-util)
> > - added tests for cases mentioned above
> > - changed usage of vshControl variables. From now on PrintToStdout calls
> > PrintToString and then prints returned string to stdout
> > 
> > Changes in v3:
> > - changed encoding of 3/3 patch, otherwise it cannot be applied
> > 
> > Changes in v2:
> > - added tests
> > - fixed alignment for unicode character which span more spaces
> > - moved ncolumns check to vshTableRowAppend
> > - changed arguments for functions vshTablePrint, vshTablePrintToStdout,
> >     vshTablePrintToString
> > 
> > Simon Kobyda (3):
> >   vsh: Add API for printing tables.
> >   virsh: Implement new table API for virsh list
> >   vsh: Added tests
> > 
> >  tests/Makefile.am            |   8 +
> >  tests/virshtest.c            |  14 +-
> >  tests/vshtabletest.c         | 377 +++++++++++++++++++++++++++++
> >  tools/Makefile.am            |   4 +-
> >  tools/virsh-domain-monitor.c |  43 ++--
> >  tools/vsh-table.c            | 449 +++++++++++++++++++++++++++++++++++
> >  tools/vsh-table.h            |  42 ++++
> >  7 files changed, 910 insertions(+), 27 deletions(-)
> >  create mode 100644 tests/vshtabletest.c
> >  create mode 100644 tools/vsh-table.c
> >  create mode 100644 tools/vsh-table.h
> > 
> 
> ACKed and pushed.
> 
> Now we can start converting the rest of the code to use vshTable APIs.

But first fix the build failures :-)

On CentOS / RHEL:

https://travis-ci.org/libvirt/libvirt/jobs/420024141


 4) testUnicode                                                       ... 
Offset 30
Expect [государство  
-----------------------------------------
 1    fedora28              running      
 2    🙊🙉🙈rhel7.5🙆🙆🙅]
Actual [                                                                                    государство  
-----------------------------------------------------------------------------------------------------------------------------
 1    fedora28                                                                                                  running      
 2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]



On Mingw:

https://travis-ci.org/libvirt/libvirt/jobs/420024142

vsh-table.c: In function 'vshTableSafeEncode':
vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' [-Werror=implicit-function-declaration]
                 *width += wcwidth(wc);
                           ^~~~~~~
vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' [-Werror=nested-externs]

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
>> On 08/23/2018 05:53 PM, Simon Kobyda wrote:
>>> Created new API for priting tables, mainly to solve alignment problems.
>>> Implemented these test to virsh list. In the future, API may be
>>> everywhere in virsh and virt-admin.
>>> Also wrote basic tests for the new API, and corrected tests in virshtest
>>> which are influenced by implementation of the API in virsh list.
>>>
>>> Changes in v5:
>>> - cleanup and merged code for calculating zero-width, nonprintable and combined
>>> character.
>>> - replaced virBufferAddStr with virBufferAddChar in some places
>>> - in tests moved code for setting correct locale
>>> - fixed few leaks and unitialized values
>>>
>>> Changes in v4:
>>> - fixed width calculation for zero-width, nonprintable and combined
>>> character. (pulled some code from linux-util)
>>> - added tests for cases mentioned above
>>> - changed usage of vshControl variables. From now on PrintToStdout calls
>>> PrintToString and then prints returned string to stdout
>>>
>>> Changes in v3:
>>> - changed encoding of 3/3 patch, otherwise it cannot be applied
>>>
>>> Changes in v2:
>>> - added tests
>>> - fixed alignment for unicode character which span more spaces
>>> - moved ncolumns check to vshTableRowAppend
>>> - changed arguments for functions vshTablePrint, vshTablePrintToStdout,
>>>     vshTablePrintToString
>>>
>>> Simon Kobyda (3):
>>>   vsh: Add API for printing tables.
>>>   virsh: Implement new table API for virsh list
>>>   vsh: Added tests
>>>
>>>  tests/Makefile.am            |   8 +
>>>  tests/virshtest.c            |  14 +-
>>>  tests/vshtabletest.c         | 377 +++++++++++++++++++++++++++++
>>>  tools/Makefile.am            |   4 +-
>>>  tools/virsh-domain-monitor.c |  43 ++--
>>>  tools/vsh-table.c            | 449 +++++++++++++++++++++++++++++++++++
>>>  tools/vsh-table.h            |  42 ++++
>>>  7 files changed, 910 insertions(+), 27 deletions(-)
>>>  create mode 100644 tests/vshtabletest.c
>>>  create mode 100644 tools/vsh-table.c
>>>  create mode 100644 tools/vsh-table.h
>>>
>>
>> ACKed and pushed.
>>
>> Now we can start converting the rest of the code to use vshTable APIs.
> 
> But first fix the build failures :-)
> 
> On CentOS / RHEL:
> 
> https://travis-ci.org/libvirt/libvirt/jobs/420024141
> 
> 
>  4) testUnicode                                                       ... 
> Offset 30
> Expect [государство  
> -----------------------------------------
>  1    fedora28              running      
>  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> Actual [                                                                                    государство  
> -----------------------------------------------------------------------------------------------------------------------------
>  1    fedora28                                                                                                  running      
>  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> 

Okay, this is probably due to ancient gcc that's there (4.8.0) and is
supposed to be fixed by adding -finput-charset= onto gcc command line.
Haven't tested it though.

> 
> 
> On Mingw:
> 
> https://travis-ci.org/libvirt/libvirt/jobs/420024142
> 
> vsh-table.c: In function 'vshTableSafeEncode':
> vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' [-Werror=implicit-function-declaration]
>                  *width += wcwidth(wc);
>                            ^~~~~~~
> vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' [-Werror=nested-externs]

But this is weird. wcwidth() manpage says to include wchar.t which we
are doing. Maybe _XOPEN_SOURCE macro is not defined?

Anyway, I'll let Simon to figure this out O:-)

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > 
> > But first fix the build failures :-)
> > 
> > On CentOS / RHEL:
> > 
> > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > 
> > 
> >  4)
> > testUnicode                                                       .
> > .. 
> > Offset 30
> > Expect [государство  
> > -----------------------------------------
> >  1    fedora28              running      
> >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > Actual
> > [                                                                  
> >                   государство  
> > -----------------------------------------------------------------
> > ------------------------------------------------------------
> >  1    fedora28                                                     
> >                                              running      
> >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > 
> 
> Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> supposed to be fixed by adding -finput-charset= onto gcc command
> line.
> Haven't tested it though.

I tried but it didn't help. From what I understood, CentOS has problems
with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert 
any of those characters to wchar_t successfully and properly, but when
we pass that character to iswprint, it returns 0 (considers those wide
characters nonprintable).

Simon Kobyda

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > 
> > > But first fix the build failures :-)
> > > 
> > > On CentOS / RHEL:
> > > 
> > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > 
> > > 
> > >  4)
> > > testUnicode                                                       .
> > > .. 
> > > Offset 30
> > > Expect [государство  
> > > -----------------------------------------
> > >  1    fedora28              running      
> > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > > Actual
> > > [                                                                  
> > >                   государство  
> > > -----------------------------------------------------------------
> > > ------------------------------------------------------------
> > >  1    fedora28                                                     
> > >                                              running      
> > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > 
> > 
> > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > supposed to be fixed by adding -finput-charset= onto gcc command
> > line.
> > Haven't tested it though.
> 
> I tried but it didn't help. From what I understood, CentOS has problems
> with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert 
> any of those characters to wchar_t successfully and properly, but when
> we pass that character to iswprint, it returns 0 (considers those wide
> characters nonprintable).

On the plus side, it appears that when this problem hits, the code is
still correctly doing the column alignment taking account of these
unexpected escape sequences.

So how about storing 2 sets of expected data for this test case.

In the unit test then call iswprint() to figure out which of the
two expected data sets to compare against.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > >
> > > > But first fix the build failures :-)
> > > >
> > > > On CentOS / RHEL:
> > > >
> > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > >
> > > >
> > > >  4)
> > > > testUnicode                                                       .
> > > > ..
> > > > Offset 30
> > > > Expect [государство
> > > > -----------------------------------------
> > > >  1    fedora28              running
> > > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > Actual
> > > > [
> > > >                   государство
> > > > -----------------------------------------------------------------
> > > > ------------------------------------------------------------
> > > >  1    fedora28
> > > >                                              running
> > > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > >
> > >
> > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > line.
> > > Haven't tested it though.
> >
> > I tried but it didn't help. From what I understood, CentOS has problems
> > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > any of those characters to wchar_t successfully and properly, but when
> > we pass that character to iswprint, it returns 0 (considers those wide
> > characters nonprintable).
>
> On the plus side, it appears that when this problem hits, the code is
> still correctly doing the column alignment taking account of these
> unexpected escape sequences.
>
> So how about storing 2 sets of expected data for this test case.
>
> In the unit test then call iswprint() to figure out which of the
> two expected data sets to compare against.

How does it help us during runtime when someone uses such characters in a
domain's name? It would still return a row consisting of escape sequences. So
what's the point of providing 2 sets of expected data for a test just so it can
pass, rather than use unicode characters we know would pass and everything else
is a platform limitation which is out of our hands.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
>On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
>> On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
>> > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
>> > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
>> > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
>> > > >
>> > > > But first fix the build failures :-)
>> > > >
>> > > > On CentOS / RHEL:
>> > > >
>> > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
>> > > >
>> > > >
>> > > >  4)
>> > > > testUnicode                                                       .
>> > > > ..
>> > > > Offset 30
>> > > > Expect [государство
>> > > > -----------------------------------------
>> > > >  1    fedora28              running
>> > > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
>> > > > Actual
>> > > > [
>> > > >                   государство
>> > > > -----------------------------------------------------------------
>> > > > ------------------------------------------------------------
>> > > >  1    fedora28
>> > > >                                              running
>> > > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
>> > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
>> > > >
>> > >
>> > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
>> > > supposed to be fixed by adding -finput-charset= onto gcc command
>> > > line.
>> > > Haven't tested it though.
>> >
>> > I tried but it didn't help. From what I understood, CentOS has problems
>> > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
>> > any of those characters to wchar_t successfully and properly, but when
>> > we pass that character to iswprint, it returns 0 (considers those wide
>> > characters nonprintable).
>>
>> On the plus side, it appears that when this problem hits, the code is
>> still correctly doing the column alignment taking account of these
>> unexpected escape sequences.
>>
>> So how about storing 2 sets of expected data for this test case.
>>

Two is not enough. My clang 5.0.1 produces a test that displays the
monkeys correctly, but does not count their width properly:

$ VIR_TEST_RANGE=4 VIR_TEST_DEBUG=1 ./run tests/vshtabletest
TEST: vshtabletest
 4) testUnicode                                                       ...
Offset 24
Expect [      государство
-----------------------------------------
 1    fedora28      ]
Actual [государство
-----------------------------------
 1    fedora28]
                                                                      ... FAILED
>> In the unit test then call iswprint() to figure out which of the
>> two expected data sets to compare against.
>
>How does it help us during runtime when someone uses such characters in a
>domain's name? It would still return a row consisting of escape sequences.

Not necessarily, see above.

>So
>what's the point of providing 2 sets of expected data for a test just so it can
>pass, rather than use unicode characters we know would pass and everything else
>is a platform limitation which is out of our hands.

I still see a benefit in having testUnicodeBasic that passes everywhere
(does it?), and conditionally running the monkey test on platforms where
iswprint returns the proper results.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > > > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > > >
> > > > > > But first fix the build failures :-)
> > > > > >
> > > > > > On CentOS / RHEL:
> > > > > >
> > > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > > > >
> > > > > >
> > > > > >  4)
> > > > > > testUnicode                                                       .
> > > > > > ..
> > > > > > Offset 30
> > > > > > Expect [государство
> > > > > > -----------------------------------------
> > > > > >  1    fedora28              running
> > > > > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > > > Actual
> > > > > > [
> > > > > >                   государство
> > > > > > -----------------------------------------------------------------
> > > > > > ------------------------------------------------------------
> > > > > >  1    fedora28
> > > > > >                                              running
> > > > > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > > > >
> > > > >
> > > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > > > line.
> > > > > Haven't tested it though.
> > > >
> > > > I tried but it didn't help. From what I understood, CentOS has problems
> > > > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > > > any of those characters to wchar_t successfully and properly, but when
> > > > we pass that character to iswprint, it returns 0 (considers those wide
> > > > characters nonprintable).
> > >
> > > On the plus side, it appears that when this problem hits, the code is
> > > still correctly doing the column alignment taking account of these
> > > unexpected escape sequences.
> > >
> > > So how about storing 2 sets of expected data for this test case.
> > >
>
> Two is not enough. My clang 5.0.1 produces a test that displays the
> monkeys correctly, but does not count their width properly:
>
> $ VIR_TEST_RANGE=4 VIR_TEST_DEBUG=1 ./run tests/vshtabletest
> TEST: vshtabletest
> 4) testUnicode                                                       ...
> Offset 24
> Expect [      государство
> -----------------------------------------
> 1    fedora28      ]
> Actual [государство
> -----------------------------------
> 1    fedora28]
>                                                                      ... FAILED
> > > In the unit test then call iswprint() to figure out which of the
> > > two expected data sets to compare against.
> >
> > How does it help us during runtime when someone uses such characters in a
> > domain's name? It would still return a row consisting of escape sequences.
>
> Not necessarily, see above.
>
> > So
> > what's the point of providing 2 sets of expected data for a test just so it can
> > pass, rather than use unicode characters we know would pass and everything else
> > is a platform limitation which is out of our hands.
>
> I still see a benefit in having testUnicodeBasic that passes everywhere
> (does it?), and conditionally running the monkey test on platforms where
> iswprint returns the proper results.

Why? To see that glibc is new enough to support it? One would assume that if it
works for n (given your example I'm so sure it actually does...), it would work
for n+1 too, so I still don't see the point in this specific test case.

Thanks for the example though.
Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:30:23PM +0200, Erik Skultety wrote:
>On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
>> I still see a benefit in having testUnicodeBasic that passes everywhere
>> (does it?), and conditionally running the monkey test on platforms where
>> iswprint returns the proper results.
>
>Why? To see that glibc is new enough to support it? One would assume

Then one might be surprised one day.

Jano

>that if it
>works for n (given your example I'm so sure it actually does...), it would work
>for n+1 too, so I still don't see the point in this specific test case.
>
>Thanks for the example though.
>Erik
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > > > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > > >
> > > > > > But first fix the build failures :-)
> > > > > >
> > > > > > On CentOS / RHEL:
> > > > > >
> > > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > > > >
> > > > > >
> > > > > >  4)
> > > > > > testUnicode                                                       .
> > > > > > ..
> > > > > > Offset 30
> > > > > > Expect [государство
> > > > > > -----------------------------------------
> > > > > >  1    fedora28              running
> > > > > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > > > Actual
> > > > > > [
> > > > > >                   государство
> > > > > > -----------------------------------------------------------------
> > > > > > ------------------------------------------------------------
> > > > > >  1    fedora28
> > > > > >                                              running
> > > > > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > > > >
> > > > >
> > > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > > > line.
> > > > > Haven't tested it though.
> > > >
> > > > I tried but it didn't help. From what I understood, CentOS has problems
> > > > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > > > any of those characters to wchar_t successfully and properly, but when
> > > > we pass that character to iswprint, it returns 0 (considers those wide
> > > > characters nonprintable).
> > > 
> > > On the plus side, it appears that when this problem hits, the code is
> > > still correctly doing the column alignment taking account of these
> > > unexpected escape sequences.
> > > 
> > > So how about storing 2 sets of expected data for this test case.
> > > 
> 
> Two is not enough. My clang 5.0.1 produces a test that displays the
> monkeys correctly, but does not count their width properly:

Is this a different bug though ? The issue with iswprint() is varying
according to glibc version, not compiler version.

So I wonder if the clang problem you mention is something that can be
fixed in some way ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
>On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
>> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
>> > > So how about storing 2 sets of expected data for this test case.
>> > >
>>
>> Two is not enough. My clang 5.0.1 produces a test that displays the
>> monkeys correctly, but does not count their width properly:
>
>Is this a different bug though ? The issue with iswprint() is varying
>according to glibc version, not compiler version.
>

The broken setup is:
sys-libs/glibc-2.25-r9
sys-devel/clang-5.0.1

It works on:
sys-libs/glibc-2.26-r7
with either of:
sys-devel/clang-5.0.1
sys-devel/clang-6.0.1

So yes, it is a glibc bug.
Depending on the version, either just wcwidth returns incorrect values
for the monkeys (my case) or iswprint considers them non-printable.

>So I wonder if the clang problem you mention is something that can be
>fixed in some way ?
>

Nope, red herring. My maint point was that there are more than 2
possible results.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > > So how about storing 2 sets of expected data for this test case.
> > > > >
> > > 
> > > Two is not enough. My clang 5.0.1 produces a test that displays the
> > > monkeys correctly, but does not count their width properly:
> > 
> > Is this a different bug though ? The issue with iswprint() is varying
> > according to glibc version, not compiler version.
> > 
> 
> The broken setup is:
> sys-libs/glibc-2.25-r9
> sys-devel/clang-5.0.1
> 
> It works on:
> sys-libs/glibc-2.26-r7
> with either of:
> sys-devel/clang-5.0.1
> sys-devel/clang-6.0.1
> 
> So yes, it is a glibc bug.
> Depending on the version, either just wcwidth returns incorrect values
> for the monkeys (my case) or iswprint considers them non-printable.

It sounds like in your case we're genuinely broken in the virsh
code, not merely tests broken.

I wonder if we need extra logic in the virsh code to handle escaping
for the cases where wcwidth is returning wrong data, so we still get
column layout correct ?

> > So I wonder if the clang problem you mention is something that can be
> > fixed in some way ?
> > 
> 
> Nope, red herring. My maint point was that there are more than 2
> possible results.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
>On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
>> On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
>> > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
>> > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
>> > > > > So how about storing 2 sets of expected data for this test case.
>> > > > >
>> > >
>> > > Two is not enough. My clang 5.0.1 produces a test that displays the
>> > > monkeys correctly, but does not count their width properly:
>> >
>> > Is this a different bug though ? The issue with iswprint() is varying
>> > according to glibc version, not compiler version.
>> >
>>
>> The broken setup is:
>> sys-libs/glibc-2.25-r9
>> sys-devel/clang-5.0.1
>>
>> It works on:
>> sys-libs/glibc-2.26-r7
>> with either of:
>> sys-devel/clang-5.0.1
>> sys-devel/clang-6.0.1
>>
>> So yes, it is a glibc bug.
>> Depending on the version, either just wcwidth returns incorrect values
>> for the monkeys (my case) or iswprint considers them non-printable.
>
>It sounds like in your case we're genuinely broken in the virsh
>code, not merely tests broken.
>
>I wonder if we need extra logic in the virsh code to handle escaping
>for the cases where wcwidth is returning wrong data, so we still get
>column layout correct ?
>

I don't think so.

1) wouldn't that involve reimplementing the wcwidth function?
2) users crazy enough to use new unicode characters are welcome to
   upgrade to new glibc, or suffer through misaligned virsh tables.

Jano

>> > So I wonder if the clang problem you mention is something that can be
>> > fixed in some way ?
>> >
>>
>> Nope, red herring. My maint point was that there are more than 2
>> possible results.
>
>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 03:52:56PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
> > On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> > > On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > > > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > > > > So how about storing 2 sets of expected data for this test case.
> > > > > > >
> > > > >
> > > > > Two is not enough. My clang 5.0.1 produces a test that displays the
> > > > > monkeys correctly, but does not count their width properly:
> > > >
> > > > Is this a different bug though ? The issue with iswprint() is varying
> > > > according to glibc version, not compiler version.
> > > >
> > >
> > > The broken setup is:
> > > sys-libs/glibc-2.25-r9
> > > sys-devel/clang-5.0.1
> > >
> > > It works on:
> > > sys-libs/glibc-2.26-r7
> > > with either of:
> > > sys-devel/clang-5.0.1
> > > sys-devel/clang-6.0.1
> > >
> > > So yes, it is a glibc bug.
> > > Depending on the version, either just wcwidth returns incorrect values
> > > for the monkeys (my case) or iswprint considers them non-printable.
> >
> > It sounds like in your case we're genuinely broken in the virsh
> > code, not merely tests broken.
> >
> > I wonder if we need extra logic in the virsh code to handle escaping
> > for the cases where wcwidth is returning wrong data, so we still get
> > column layout correct ?
> >
>
> I don't think so.
>
> 1) wouldn't that involve reimplementing the wcwidth function?
> 2) users crazy enough to use new unicode characters are welcome to
>   upgrade to new glibc, or suffer through misaligned virsh tables.

I second that, I don't think that in this case it's any beneficial trying to
fix glibc problems just to offer users a consistent experience, we're not
gnulib, besides, we're talking about table alignment which has been broken
for ages and we've already made a significant progress here. Additionally, as
Jano has pointed out, if someone feels adventurous, that's fine, but it
may come with certain limitations.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 04:10:53PM +0200, Erik Skultety wrote:
> On Tue, Aug 28, 2018 at 03:52:56PM +0200, Ján Tomko wrote:
> > On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
> > > On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> > > > On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> > > > > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > > > > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > > > > > So how about storing 2 sets of expected data for this test case.
> > > > > > > >
> > > > > >
> > > > > > Two is not enough. My clang 5.0.1 produces a test that displays the
> > > > > > monkeys correctly, but does not count their width properly:
> > > > >
> > > > > Is this a different bug though ? The issue with iswprint() is varying
> > > > > according to glibc version, not compiler version.
> > > > >
> > > >
> > > > The broken setup is:
> > > > sys-libs/glibc-2.25-r9
> > > > sys-devel/clang-5.0.1
> > > >
> > > > It works on:
> > > > sys-libs/glibc-2.26-r7
> > > > with either of:
> > > > sys-devel/clang-5.0.1
> > > > sys-devel/clang-6.0.1
> > > >
> > > > So yes, it is a glibc bug.
> > > > Depending on the version, either just wcwidth returns incorrect values
> > > > for the monkeys (my case) or iswprint considers them non-printable.
> > >
> > > It sounds like in your case we're genuinely broken in the virsh
> > > code, not merely tests broken.
> > >
> > > I wonder if we need extra logic in the virsh code to handle escaping
> > > for the cases where wcwidth is returning wrong data, so we still get
> > > column layout correct ?
> > >
> >
> > I don't think so.
> >
> > 1) wouldn't that involve reimplementing the wcwidth function?
> > 2) users crazy enough to use new unicode characters are welcome to
> >   upgrade to new glibc, or suffer through misaligned virsh tables.
> 
> I second that, I don't think that in this case it's any beneficial trying to
> fix glibc problems just to offer users a consistent experience, we're not
> gnulib, besides, we're talking about table alignment which has been broken
> for ages and we've already made a significant progress here. Additionally, as
> Jano has pointed out, if someone feels adventurous, that's fine, but it
> may come with certain limitations.

Ok, so we just need get the test to correctly skip on platforms where
we know it won't get right answers


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 08/28/2018 10:11 AM, Daniel P. Berrangé wrote:
> On Tue, Aug 28, 2018 at 04:10:53PM +0200, Erik Skultety wrote:
>> On Tue, Aug 28, 2018 at 03:52:56PM +0200, Ján Tomko wrote:
>>> On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
>>>> On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
>>>>> On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
>>>>>> On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
>>>>>>> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
>>>>>>>>> So how about storing 2 sets of expected data for this test case.
>>>>>>>>>
>>>>>>>
>>>>>>> Two is not enough. My clang 5.0.1 produces a test that displays the
>>>>>>> monkeys correctly, but does not count their width properly:
>>>>>>
>>>>>> Is this a different bug though ? The issue with iswprint() is varying
>>>>>> according to glibc version, not compiler version.
>>>>>>
>>>>>
>>>>> The broken setup is:
>>>>> sys-libs/glibc-2.25-r9
>>>>> sys-devel/clang-5.0.1
>>>>>
>>>>> It works on:
>>>>> sys-libs/glibc-2.26-r7
>>>>> with either of:
>>>>> sys-devel/clang-5.0.1
>>>>> sys-devel/clang-6.0.1
>>>>>
>>>>> So yes, it is a glibc bug.
>>>>> Depending on the version, either just wcwidth returns incorrect values
>>>>> for the monkeys (my case) or iswprint considers them non-printable.
>>>>
>>>> It sounds like in your case we're genuinely broken in the virsh
>>>> code, not merely tests broken.
>>>>
>>>> I wonder if we need extra logic in the virsh code to handle escaping
>>>> for the cases where wcwidth is returning wrong data, so we still get
>>>> column layout correct ?
>>>>
>>>
>>> I don't think so.
>>>
>>> 1) wouldn't that involve reimplementing the wcwidth function?
>>> 2) users crazy enough to use new unicode characters are welcome to
>>>   upgrade to new glibc, or suffer through misaligned virsh tables.
>>
>> I second that, I don't think that in this case it's any beneficial trying to
>> fix glibc problems just to offer users a consistent experience, we're not
>> gnulib, besides, we're talking about table alignment which has been broken
>> for ages and we've already made a significant progress here. Additionally, as
>> Jano has pointed out, if someone feels adventurous, that's fine, but it
>> may come with certain limitations.
> 
> Ok, so we just need get the test to correctly skip on platforms where
> we know it won't get right answers
> 
> 

Cannot believe the effort/time on this...

The problem with bypassing on such platforms is how does one then know
those platforms get fixed so that the "skip" can be removed? Of course
seeing test failure is not good either.

There is no simple answer, we can try to code around this or we can
accept on certain platforms the specific test may fail for a specific
reason and move on.

In the long run, it's a format that what percentage of our users will
use? Is naming domains or "other" libvirt objects using emojis a real
world problem that we really care to ensure that the table lines up
properly? Maybe it's just best to determine that the name has one of
those and accept that calculations are going to be wrong. Maybe we just
decide if we see one in a name (e.g. a non normally printable or greater
than some charset value via 0xNNNN) that we "assume" or "assign" the
maximum width possible for each of those characters found and move on.

Maybe we should "Validate" that a name doesn't have those chars and if
it does fail to recognize using a new errno code we invent, "ENO🙊🙉🙈,
usage of emoticons as names or keys is not supportable".

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > >
> > > > > But first fix the build failures :-)
> > > > >
> > > > > On CentOS / RHEL:
> > > > >
> > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > > >
> > > > >
> > > > >  4)
> > > > > testUnicode                                                       .
> > > > > ..
> > > > > Offset 30
> > > > > Expect [государство
> > > > > -----------------------------------------
> > > > >  1    fedora28              running
> > > > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > > Actual
> > > > > [
> > > > >                   государство
> > > > > -----------------------------------------------------------------
> > > > > ------------------------------------------------------------
> > > > >  1    fedora28
> > > > >                                              running
> > > > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > > >
> > > >
> > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > > line.
> > > > Haven't tested it though.
> > >
> > > I tried but it didn't help. From what I understood, CentOS has problems
> > > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > > any of those characters to wchar_t successfully and properly, but when
> > > we pass that character to iswprint, it returns 0 (considers those wide
> > > characters nonprintable).
> >
> > On the plus side, it appears that when this problem hits, the code is
> > still correctly doing the column alignment taking account of these
> > unexpected escape sequences.
> >
> > So how about storing 2 sets of expected data for this test case.
> >
> > In the unit test then call iswprint() to figure out which of the
> > two expected data sets to compare against.
> 
> How does it help us during runtime when someone uses such characters in a
> domain's name? It would still return a row consisting of escape sequences. So
> what's the point of providing 2 sets of expected data for a test just so it can
> pass, rather than use unicode characters we know would pass and everything else
> is a platform limitation which is out of our hands.

Well the idea is to prove that we get the column width calculation
correct, even when the the glibc we have doesn't support these as
printable characters

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Aug 24, 2018 at 12:10:47PM +0200, Michal Privoznik wrote:
> On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> >> On 08/23/2018 05:53 PM, Simon Kobyda wrote:
> >>> Created new API for priting tables, mainly to solve alignment problems.
> >>> Implemented these test to virsh list. In the future, API may be
> >>> everywhere in virsh and virt-admin.
> >>> Also wrote basic tests for the new API, and corrected tests in virshtest
> >>> which are influenced by implementation of the API in virsh list.
> >>>
> >>> Changes in v5:
> >>> - cleanup and merged code for calculating zero-width, nonprintable and combined
> >>> character.
> >>> - replaced virBufferAddStr with virBufferAddChar in some places
> >>> - in tests moved code for setting correct locale
> >>> - fixed few leaks and unitialized values
> >>>
> >>> Changes in v4:
> >>> - fixed width calculation for zero-width, nonprintable and combined
> >>> character. (pulled some code from linux-util)
> >>> - added tests for cases mentioned above
> >>> - changed usage of vshControl variables. From now on PrintToStdout calls
> >>> PrintToString and then prints returned string to stdout
> >>>
> >>> Changes in v3:
> >>> - changed encoding of 3/3 patch, otherwise it cannot be applied
> >>>
> >>> Changes in v2:
> >>> - added tests
> >>> - fixed alignment for unicode character which span more spaces
> >>> - moved ncolumns check to vshTableRowAppend
> >>> - changed arguments for functions vshTablePrint, vshTablePrintToStdout,
> >>>     vshTablePrintToString
> >>>
> >>> Simon Kobyda (3):
> >>>   vsh: Add API for printing tables.
> >>>   virsh: Implement new table API for virsh list
> >>>   vsh: Added tests
> >>>
> >>>  tests/Makefile.am            |   8 +
> >>>  tests/virshtest.c            |  14 +-
> >>>  tests/vshtabletest.c         | 377 +++++++++++++++++++++++++++++
> >>>  tools/Makefile.am            |   4 +-
> >>>  tools/virsh-domain-monitor.c |  43 ++--
> >>>  tools/vsh-table.c            | 449 +++++++++++++++++++++++++++++++++++
> >>>  tools/vsh-table.h            |  42 ++++
> >>>  7 files changed, 910 insertions(+), 27 deletions(-)
> >>>  create mode 100644 tests/vshtabletest.c
> >>>  create mode 100644 tools/vsh-table.c
> >>>  create mode 100644 tools/vsh-table.h
> >>>
> >>
> >> ACKed and pushed.
> >>
> >> Now we can start converting the rest of the code to use vshTable APIs.
> > 
> > But first fix the build failures :-)
> > 
> > On CentOS / RHEL:
> > 
> > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > 
> > 
> >  4) testUnicode                                                       ... 
> > Offset 30
> > Expect [государство  
> > -----------------------------------------
> >  1    fedora28              running      
> >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > Actual [                                                                                    государство  
> > -----------------------------------------------------------------------------------------------------------------------------
> >  1    fedora28                                                                                                  running      
> >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > 
> 
> Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> supposed to be fixed by adding -finput-charset= onto gcc command line.
> Haven't tested it though.
> 
> > 
> > 
> > On Mingw:
> > 
> > https://travis-ci.org/libvirt/libvirt/jobs/420024142
> > 
> > vsh-table.c: In function 'vshTableSafeEncode':
> > vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' [-Werror=implicit-function-declaration]
> >                  *width += wcwidth(wc);
> >                            ^~~~~~~
> > vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' [-Werror=nested-externs]
> 
> But this is weird. wcwidth() manpage says to include wchar.t which we
> are doing. Maybe _XOPEN_SOURCE macro is not defined?

It is quite simple - the function doesn't exist on mingw :-)

  $ grep -r wcwidth /usr/i686-w64-mingw32/sys-root/mingw/include/
  ...nothing...

Looks like gnulib can rescue us though

https://www.gnu.org/software/gnulib/manual/html_node/wcwidth.html

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, 2018-08-24 at 11:18 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 24, 2018 at 12:10:47PM +0200, Michal Privoznik wrote:
> > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > On 08/23/2018 05:53 PM, Simon Kobyda wrote:
> > > > > Created new API for priting tables, mainly to solve alignment
> > > > > problems.
> > > > > Implemented these test to virsh list. In the future, API may
> > > > > be
> > > > > everywhere in virsh and virt-admin.
> > > > > Also wrote basic tests for the new API, and corrected tests
> > > > > in virshtest
> > > > > which are influenced by implementation of the API in virsh
> > > > > list.
> > > > > 
> > > > > Changes in v5:
> > > > > - cleanup and merged code for calculating zero-width,
> > > > > nonprintable and combined
> > > > > character.
> > > > > - replaced virBufferAddStr with virBufferAddChar in some
> > > > > places
> > > > > - in tests moved code for setting correct locale
> > > > > - fixed few leaks and unitialized values
> > > > > 
> > > > > Changes in v4:
> > > > > - fixed width calculation for zero-width, nonprintable and
> > > > > combined
> > > > > character. (pulled some code from linux-util)
> > > > > - added tests for cases mentioned above
> > > > > - changed usage of vshControl variables. From now on
> > > > > PrintToStdout calls
> > > > > PrintToString and then prints returned string to stdout
> > > > > 
> > > > > Changes in v3:
> > > > > - changed encoding of 3/3 patch, otherwise it cannot be
> > > > > applied
> > > > > 
> > > > > Changes in v2:
> > > > > - added tests
> > > > > - fixed alignment for unicode character which span more
> > > > > spaces
> > > > > - moved ncolumns check to vshTableRowAppend
> > > > > - changed arguments for functions vshTablePrint,
> > > > > vshTablePrintToStdout,
> > > > >     vshTablePrintToString
> > > > > 
> > > > > Simon Kobyda (3):
> > > > >   vsh: Add API for printing tables.
> > > > >   virsh: Implement new table API for virsh list
> > > > >   vsh: Added tests
> > > > > 
> > > > >  tests/Makefile.am            |   8 +
> > > > >  tests/virshtest.c            |  14 +-
> > > > >  tests/vshtabletest.c         | 377
> > > > > +++++++++++++++++++++++++++++
> > > > >  tools/Makefile.am            |   4 +-
> > > > >  tools/virsh-domain-monitor.c |  43 ++--
> > > > >  tools/vsh-table.c            | 449
> > > > > +++++++++++++++++++++++++++++++++++
> > > > >  tools/vsh-table.h            |  42 ++++
> > > > >  7 files changed, 910 insertions(+), 27 deletions(-)
> > > > >  create mode 100644 tests/vshtabletest.c
> > > > >  create mode 100644 tools/vsh-table.c
> > > > >  create mode 100644 tools/vsh-table.h
> > > > > 
> > > > 
> > > > ACKed and pushed.
> > > > 
> > > > Now we can start converting the rest of the code to use
> > > > vshTable APIs.
> > > 
> > > But first fix the build failures :-)
> > > 
> > > On CentOS / RHEL:
> > > 
> > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > 
> > > 
> > >  4)
> > > testUnicode                                                      
> > >  ... 
> > > Offset 30
> > > Expect [государство  
> > > -----------------------------------------
> > >  1    fedora28              running      
> > >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > > Actual
> > > [                                                                
> > >                     государство  
> > > ---------------------------------------------------------------
> > > --------------------------------------------------------------
> > >  1    fedora28                                                   
> > >                                                running      
> > >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff
> > > \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > 
> > 
> > Okay, this is probably due to ancient gcc that's there (4.8.0) and
> > is
> > supposed to be fixed by adding -finput-charset= onto gcc command
> > line.
> > Haven't tested it though.
> > 
> > > 
> > > 
> > > On Mingw:
> > > 
> > > https://travis-ci.org/libvirt/libvirt/jobs/420024142
> > > 
> > > vsh-table.c: In function 'vshTableSafeEncode':
> > > vsh-table.c:274:27: error: implicit declaration of function
> > > 'wcwidth' [-Werror=implicit-function-declaration]
> > >                  *width += wcwidth(wc);
> > >                            ^~~~~~~
> > > vsh-table.c:274:27: error: nested extern declaration of 'wcwidth'
> > > [-Werror=nested-externs]
> > 
> > But this is weird. wcwidth() manpage says to include wchar.t which
> > we
> > are doing. Maybe _XOPEN_SOURCE macro is not defined?
> 
> It is quite simple - the function doesn't exist on mingw :-)
> 
>   $ grep -r wcwidth /usr/i686-w64-mingw32/sys-root/mingw/include/
>   ...nothing...
> 
> Looks like gnulib can rescue us though
> 
> https://www.gnu.org/software/gnulib/manual/html_node/wcwidth.html
> 
> Regards,
> Daniel

So I've looked it up and perhaps found something. Function iswprint
(which is the root of our problem) is provided by glibc. CentOS 7 uses
glibc 2.17, which was released in December 2017. I looked into
ChangeLog of that version and I see they added table (or map?) for
their nonascii symbols in 2009. Unicodes I used in tests (monkeyfaces)
were added in 2010 in Unicode 6.0. So those specific unicodes therefore
didn't find their way into nonascii table of glibc 2.17. :).
I suspect the case is similar for RHEL.
Btw. iswprint is using that table to determine whetever character is
printable or not.

Simon Kobyda.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-08-28 at 14:35 +0200, Simon Kobyda wrote:
> CentOS 7 uses
> glibc 2.17, which was released in December 2017

Sorry made a typo there, glibc 2.17 was released in 2012

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list