Ticket 2791

On 2016-06-01, Tim Wickberg wrote:

On I've looked into this, and it's not as bad as it seems at first glance. Enhancement bug 2265 covers some of this thinking, from the standpoint of preventing malicious junk from wrecking other users' (or sysadmins') terminals by feeding control characters into job names.

wchar_t is considered to have been a mistake by a lot of people. [1] char32_t is not much better - there are now Unicode sequences that don't fix 32-bits now.

UTF8 is almost always easier to work with - most of the same char routines will naively handle this (and Slurm already naively supports UTF8 through this mechanism - hence my occasional testing of a node named ☃) - with the exception of multi-byte characters being inadvertently truncated when using fixed-width output formatting.

"Proper" UTF8 support wouldn't be too difficult to handle - for each string it would require calculating the display-width versus characters used, and also need to prevent splitting multi-byte characters when truncating any strings. The backend code doesn't need to change (with the possible exception of some debug messages) since slurmctld leaves the full character array intact.

Since most of the output formatting routines are shared, it may not be hard to add, although we'd need to import a library ([2] seems like a good option) to give us a way to calculate utf8 string-widths.

[1] http://utf8everywhere.org/
[2] https://github.com/JuliaLang/utf8proc

FWIW, I have a preliminary patch for this. I'll clean it up a bit and wire up more of the user commands (so far I have tested only squeue, specifically the user comment field) before submitting a proper patch.

I've change this bug to severity 4 / contribution

Please attach your patch when you are ready to share it.

Created attachment 4752 [details]
Handle column width when printing UTF-8

Here's the patch, in git format-patch format including a hopefully useful commit msg.

I haven't used any external libraries except functionality which should be present in any C99/POSIX.1-2001 libc. Since AFAIU this is more or less required for slurm in general, I haven't done any autoconf tests for the functions either and just assume that they are present.

This uses the much-maligned wchar_t, but only in the internals of a few helper functions, and thus does not pollute the slurm API.

ping!?

I've looked at it briefly, but as you'd correctly surmised I'm not especially enthused by the use of wchar_t and reliance on <locale.h>. But I haven't come up with a better explanation than "it feels wrong". Given the rest of our team's silence on this I think they're in the same category currently.

My current preference would be to use libutf8proc or similar, and use it if available at configure time (then fall back to a naive strlen call if unavailable). I think that could be used with a bit of care along with a modified version of your patch if you're interested in exploring that.

Updating some meta records here, and moving back to an enhancement request for now.