On 07-May-21 3:13 AM, Richael Zhuang wrote:

quoted

@@ -262,18 +189,16 @@ static int
  power_init_for_setting_freq(struct acpi_power_info *pi)  {
  	FILE *f;
-	char fullpath[PATH_MAX];
  	char buf[BUFSIZ];
  	uint32_t i, freq;
-	char *s;
+	int ret;

-	snprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_SETSPEED,
-			pi->lcore_id);
-	f = fopen(fullpath, "rw+");
-	FOPEN_OR_ERR_RET(f, -1);
+	open_core_sysfs_file(POWER_SYSFILE_SETSPEED, pi->lcore_id, "r",
+			&f);

Hi Anatoly,
I tried to verify your patch on my arm platform,  and I found several bugs.

Here it should be "rw+", for it will write freq to POWER_SYSFILE_SETSPEED later.

Best Regards,
Richael

<snip>

quoted

  	return 1;
  }
+
+int
+open_core_sysfs_file(const char *template, unsigned int core, const char
*mode,
+		FILE **f)
+{
+	char fullpath[PATH_MAX];
+	FILE *tmpf;
+
+	/* silenced -Wformat-nonliteral here */
+	snprintf(fullpath, sizeof(fullpath), template, core);
+	tmpf = fopen(fullpath, mode);
+	if (tmpf == NULL)
+		return -1;
+	*f = tmpf;

When the file that open_core_sysfs_file() opens doesn't exist, there's segmentation fault.
Moving *f=tmpf above runs OK.

quoted

+
+	return 0;
+}
+
+int
+read_core_sysfs_u32(FILE *f, uint32_t *val) {
+	char buf[BUFSIZ];
+	uint32_t fval;
+	char *s;
+
+	s = fgets(buf, sizeof(buf), f);
+	if (s == NULL)
+		return -1;
+

<snip>

quoted

+	/* strip off any terminating newlines */
+	*strchrnul(buf, '\n') = '\0';
+
+	return 0;
+}
+
+int
+write_core_sysfs_s(FILE *f, const char *str) {
+	int ret;
+
+	ret = fseek(f, 0, SEEK_SET);
+	if (ret != 0)
+		return -1;
+
+	ret = fputs(str, f);
+	if (ret != 0)

ret >=0 if success, EOF means failure.

Hi Richael,

Thank you very much for testing! I'll address these issues in the next 
revision, and double-check everything else.

-- 
Thanks,
Anatoly