Re: [Powertop] [po] Updated zh_CN translation

* Re: [Powertop] [po] Updated zh_CN translation
@ 2018-03-03  2:04 arthur2e5
  0 siblings, 0 replies; 3+ messages in thread
From: arthur2e5 @ 2018-03-03  2:04 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

On 2018-03-02 18:33, Mingcong Bai wrote:
> 
> We are currently attempting on a fix to this issue, and current
> progress may be followed here: https://github.com/AOSC-Dev/powertop/.

Hi folks,

I think I have figured out the fix, but I can't be sure about the 
patches
until Bai does a test compilation given how messy the strlen() calls 
are.
Help yourself with the attachments if you want to see them this early. I 
will
do a more "conventional" email patch-thread after Bai tests it.

Those are fairly mechanical tasks, so take it as whatever license you 
want.
For functions added/rewritten entirely on my own, consider them WTFPL.
Amend the changes as you like.

By the way, please remember to update your LINGUAS file so make knows to
compile and ship the translations. Bai did that in:

1. https://github.com/AOSC-Dev/powertop/commit/ca4b2d9
2. https://github.com/AOSC-Dev/powertop/commit/d9ccc30
3. https://github.com/AOSC-Dev/powertop/commit/9347162 (necessary evil…)

--
Best Regards,
Mingye Wang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Stop-using-strlen-on-C-strings.patch --]
[-- Type: text/x-diff, Size: 2043 bytes --]

From a1a11bae5240ef9c78ad56d57f6681225c3d4cec Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Fri, 2 Mar 2018 18:40:40 -0500
Subject: [PATCH 1/2] Stop using strlen() on C++ strings

C++'s strings have a quicker length operation. There's no need to
convert those strings to null-terminated C strings and run strlen()
on it.
---
 src/devices/ahci.cpp | 2 +-
 src/display.cpp      | 7 +++----
 src/lib.cpp          | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
index 2fa2f52..31a42d3 100644
--- a/src/devices/ahci.cpp
+++ b/src/devices/ahci.cpp
@@ -150,7 +150,7 @@ ahci::ahci(char *_name, char *path): device()
 
 	diskname = model_name(path, _name);
 
-	if (strlen(diskname.c_str()) == 0)
+	if (diskname.length() == 0)
 		snprintf(humanname, sizeof(humanname), _("SATA link: %s"), _name);
 	else
 		snprintf(humanname, sizeof(humanname), _("SATA disk: %s"), diskname.c_str());
diff --git a/src/display.cpp b/src/display.cpp
index 10ee48a..8484683 100644
--- a/src/display.cpp
+++ b/src/display.cpp
@@ -98,7 +98,7 @@ void show_tab(unsigned int tab)
 	class tab_window *win;
 	unsigned int i;
 	int tab_pos = 17;
-	const char *c;
+	string& s = bottom_lines[tab_names[tab]];
 
 	if (!display)
 		return;
@@ -123,9 +123,8 @@ void show_tab(unsigned int tab)
 	wattrset(bottom_line, A_REVERSE);
 	mvwprintw(bottom_line, 0,0, "%120s", "");
 
-	c = bottom_lines[tab_names[tab]].c_str();
-	if (c && strlen(c) > 0)
-		mvwprintw(bottom_line, 0,0, c);
+	if (s.length() > 0)
+		mvwprintw(bottom_line, 0,0, s.c_str());
 	else
 		mvwprintw(bottom_line, 0, 0,
 			"<ESC> %s | <TAB> / <Shift + TAB> %s | ", _("Exit"),
diff --git a/src/lib.cpp b/src/lib.cpp
index 5e48f37..b80ad36 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -424,7 +424,7 @@ char *pretty_print(const char *str, char *buf, int len)
 
 	p = pretty_prints[str].c_str();
 
-	if (strlen(p) == 0)
+	if (pretty_prints[str].length() == 0)
 		p = str;
 
 	snprintf(buf, len,  "%s", p);
-- 
2.16.1.windows.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Align-multi-column-characters-properly.patch --]
[-- Type: text/x-diff, Size: 9540 bytes --]

From 645e5893dea21c78f5ba546a54b9760dd8ebbbf5 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Fri, 2 Mar 2018 20:39:46 -0500
Subject: [PATCH 2/2] Align multi-column characters properly

Individual characters used in localization may occupy multiple bytes per
character in memory and multiple columns per character on screen. This
commit introduces facilities to do those tasks properly, and corrects
such invocations on translatable strings.

Change breakdown by file:

* lib.{cpp,h}:
	Introduce a measure_string function as a wcswidth() wrapper.
		This function converts a char* to wint_t* and calls
		wcswidth.
	Modify align_string to count columns properly.
	Provide C++ string variants of both functions.
* display.cpp:
	Use measure_string for tab position.
	Measure the printed translation, not the original name.
* cpu:
	Replace expand_string with append_string_expanded_cpp:
		Append char* to the string given,
		calculate the padding needed by measuring the char*,
		and pad the char*.
	Replace calls accordingly.
* devices/device.cpp, tuning/tuning.cpp:
	Replace while loops with align_string.
---
 src/cpu/cpu.cpp        | 60 ++++++++++++++++++++++----------------
 src/devices/device.cpp |  2 +-
 src/display.cpp        |  2 +-
 src/lib.cpp            | 78 +++++++++++++++++++++++++++++++++++++++++++-------
 src/lib.h              |  6 ++++
 src/tuning/tuning.cpp  |  7 ++---
 6 files changed, 115 insertions(+), 40 deletions(-)

diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index f3711f5..7ef1abd 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -333,10 +333,17 @@ void end_cpu_measurement(void)
 	perf_events->stop();
 }
 
-static void expand_string(char *string, unsigned int newlen)
+static void append_string_expanded_cpp(string &s, const char* more, unsigned int add_width)
 {
-	while (strlen(string) < newlen)
-		strcat(string, " ");
+	s.append(more);
+
+	int width = measure_string(more);
+	if (width < 0)
+		return;
+
+	int spaces_to_pad = add_width - width;
+	if (spaces_to_pad > 0)
+		s.resize(s.length() + spaces_to_add, ' ');
 }
 
 static int has_state_level(class abstract_cpu *acpu, int state, int line)
@@ -826,11 +833,11 @@ void impl_w_display_cpu_states(int state)
 {
 	WINDOW *win;
 	char buffer[128];
-	char linebuf[1024];
+	string linebuf;
 	unsigned int package, core, cpu;
 	int line, loop, cstates_num, pstates_num;
 	class abstract_cpu *_package, * _core, * _cpu;
-	int ctr = 0;
+	int ctr = 0; // column count
 	unsigned int i;
 
 	cstates_num = get_cstates_num();
@@ -872,32 +879,37 @@ void impl_w_display_cpu_states(int state)
 			for (line = LEVEL_HEADER; line <= loop; line++) {
 				int first = 1;
 				ctr = 0;
-				linebuf[0] = 0;
+				linebuf.resize(0);
 
 				if (!has_state_level(_package, state, line))
 					continue;
 
 				buffer[0] = 0;
 				if (first_pkg == 0) {
-					strcat(linebuf, fill_state_name(_package, state, line, buffer));
-					expand_string(linebuf, ctr + 10);
-					strcat(linebuf, fill_state_line(_package, state, line, buffer));
+					fill_state_name(_package, state, line, buffer);
+					append_string_expanded_cpp(linebuf, buffer, 10);
+
+					/* fortunately, fill_state_line always clears buffer. */
+					fill_state_line(_package, state, line, buffer);
+					append_string_expanded_cpp(linebuf, buffer, 10);
+				} else {
+					linebuf.resize(20, ' ');
 				}
-				ctr += 20;
-				expand_string(linebuf, ctr);
+				ctr = 20;
 
-				strcat(linebuf, "| ");
+				linebuf.append("| ");
 				ctr += strlen("| ");
 
 				if (!_core->can_collapse()) {
 					buffer[0] = 0;
-					strcat(linebuf, fill_state_name(_core, state, line, buffer));
-					expand_string(linebuf, ctr + 10);
-					strcat(linebuf, fill_state_line(_core, state, line, buffer));
-					ctr += 20;
-					expand_string(linebuf, ctr);
+					fill_state_name(_core, state, line, buffer);
+					append_string_expanded_cpp(linebuf, buffer, 10);
+
+					fill_state_line(_core, state, line, buffer);
+					append_string_expanded_cpp(linebuf, buffer, 10);
 
-					strcat(linebuf, "| ");
+					ctr += 12;
+					linebuf.append("| ");
 					ctr += strlen("| ");
 				}
 
@@ -907,19 +919,19 @@ void impl_w_display_cpu_states(int state)
 						continue;
 
 					if (first == 1) {
-						strcat(linebuf, fill_state_name(_cpu, state, line, buffer));
-						expand_string(linebuf, ctr + 10);
+						fill_state_name(_core, state, line, buffer);
+						append_string_expanded_cpp(linebuf, buffer, 12);
 						first = 0;
 						ctr += 12;
 					}
 					buffer[0] = 0;
-					strcat(linebuf, fill_state_line(_cpu, state, line, buffer));
+					fill_state_line(_core, state, line, buffer);
+					append_string_expanded_cpp(linebuf, buffer, 10);
 					ctr += 10;
-					expand_string(linebuf, ctr);
 
 				}
-				strcat(linebuf, "\n");
-				wprintw(win, "%s", linebuf);
+				linebuf.append("\n");
+				wprintw(win, "%s", linebuf.c_str());
 			}
 			wprintw(win, "\n");
 			first_pkg++;
diff --git a/src/devices/device.cpp b/src/devices/device.cpp
index 2245b65..c8f64ec 100644
--- a/src/devices/device.cpp
+++ b/src/devices/device.cpp
@@ -199,7 +199,7 @@ void report_devices(void)
 			else
 				sprintf(util, "%5i%s",  (int)all_devices[i]->utilization(),  all_devices[i]->util_units());
 		}
-		while (strlen(util) < 13) strcat(util, " ");
+		align_string(util, 13, 128);
 
 		P = all_devices[i]->power_usage(&all_results, &all_parameters);
 
diff --git a/src/display.cpp b/src/display.cpp
index 8484683..484f764 100644
--- a/src/display.cpp
+++ b/src/display.cpp
@@ -140,7 +140,7 @@ void show_tab(unsigned int tab)
 				wattrset(tab_bar, A_REVERSE);
 			mvwprintw(tab_bar, 0, tab_pos, " %s ", tab_translations[tab_names[i]].c_str());
 
-			tab_pos += 3 + tab_names[i].length();
+			tab_pos += 3 + measure_string_cpp(tab_translations[tab_names[i]]);
 	}
 
 	wrefresh(tab_bar);
diff --git a/src/lib.cpp b/src/lib.cpp
index b80ad36..d16b1ea 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -23,6 +23,7 @@
  *	Arjan van de Ven <arjan@linux.intel.com>
  *	Peter Anvin
  */
+#define _XOPEN_SOURCE
 #include <map>
 #include <string.h>
 #include <iostream>
@@ -262,23 +263,80 @@ string read_sysfs_string(const char *format, const char *param)
 
 void align_string(char *buffer, size_t min_sz, size_t max_sz)
 {
-	size_t sz;
+	int width = measure_string(buffer, max_sz);
+
+	/* This may be incorrect, but remains a reasonable fallback. */
+	if (width < 0) {
+		buffer[min_sz] = 0x00;
+		return;
+	}
+
+	if (width < min_sz) {
+		int chars_to_pad = min_sz - width;
+		int len = strlen(buffer);
+
+		/* Bad input... shrug it off. */
+		if (len > max_sz) {
+			buffer[max_sz] = 0x00;
+			return;
+		}
+
+		/* Compromise to avoid out-of-bound writes */
+		if (chars_to_pad + len > max_sz)
+			chars_to_pad = max_sz - len - 1;
+
+		memset(buffer + len, ' ', chars_to_pad);
+		buffer[len + chars_to_pad] = 0x00;
+	}
+}
+
+void align_string_cpp(string &s, size_t min_sz)
+{
+	int width = measure_string_cpp(buffer);
+	/* This may be incorrect, but remains a reasonable fallback. */
+	if (width < 0) {
+		s.resize(min_sz);
+	}
+
+	if (width < min_sz) {
+		int chars_to_pad = sz - width;
+		s.resize(s.length() + chars_to_pad, ' ');
+	}
+}
+
+int measure_string(const char* cs, size_t len) {
+	wint_t *buf;
+	size_t wlen = 0;
+	int ret = -2; /* wcswidth returns -1 on non-prints, use a different ret for error */
 
 	/** mbsrtowcs() allows NULL dst and zero sz,
 	 * comparing to mbstowcs(), which causes undefined
-	 * behaviour under given circumstances*/
+	 * behaviour under given circumstances
+	 * [citation needed] */
 
 	/* start with mbsrtowcs() local mbstate_t * and
 	 * NULL dst pointer*/
-	sz = mbsrtowcs(NULL, (const char **)&buffer, max_sz, NULL);
-	if (sz == (size_t)-1) {
-		buffer[min_sz] = 0x00;
-		return;
-	}
-	while (sz < min_sz) {
-		strcat(buffer, " ");
-		sz++;
+	mbstate_t state;
+	memset(&state, 0x00, sizeof(state));
+
+	if (!cs)
+		return ret;
+
+	if (len == (size_t)-1)
+		len = strlen(cs);
+
+	buf = malloc(len * sizeof(wint_t));
+	if (!buf)
+		return ret;
+
+	wlen = mbsrtowcs(buf, cs, len, &state);
+	if (wlen == (size_t)-1) {
+		free(buf);
+		return -3; /* align_string should know it */
 	}
+
+	ret = wcswidth(buf, wlen);
+	return ret;
 }
 
 void format_watts(double W, char *buffer, unsigned int len)
diff --git a/src/lib.h b/src/lib.h
index 6d85eb6..ff4833a 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -88,6 +88,12 @@ extern int read_msr(int cpu, uint64_t offset, uint64_t *value);
 extern int write_msr(int cpu, uint64_t offset, uint64_t value);
 
 extern void align_string(char *buffer, size_t min_sz, size_t max_sz);
+extern void align_string_cpp(string &s, size_t min_sz);
+extern int measure_string(const char* cs, size_t len = -1);
+
+inline int measure_string_cpp(const string& s) {
+	return measure_string(s.c_str(), s.length());
+}
 
 extern void ui_notify_user_ncurses(const char *frmt, ...);
 extern void ui_notify_user_console(const char *frmt, ...);
diff --git a/src/tuning/tuning.cpp b/src/tuning/tuning.cpp
index 005bb4b..bf344a2 100644
--- a/src/tuning/tuning.cpp
+++ b/src/tuning/tuning.cpp
@@ -115,11 +115,10 @@ static void __tuning_update_display(int cursor_pos)
 		char desc[4096];
 		pt_strcpy(res, all_tunables[i]->result_string());
 		pt_strcpy(desc, all_tunables[i]->description());
-		while (strlen(res) < 12)
-			strcat(res, " ");
 
-		while (strlen(desc) < 103)
-			strcat(desc, " ");
+		align_string(res, 12, 128);
+		align_string(desc, 103, 128);
+
 		if ((int)i != cursor_pos) {
 			wattrset(win, A_NORMAL);
 			wprintw(win, "   ");
-- 
2.16.1.windows.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread