Kobo: Always use open/write/close for sysfs writes (#9635)

Also simplifies a few UIManager log messages (re: https://github.com/koreader/koreader-base/pull/1537)

base

@@ -1 +1 @@
-Subproject commit 4216c40d662660c9bbc48e79893b437e97518254
+Subproject commit 37506f4d552e9ccf2f6c0f71afe925c3ffd9abd2

frontend/device/kobo/device.lua

@@ -81,8 +81,8 @@ local function writeToSys(val, file)
         logger.err("Cannot open file `" .. file .. "`:", ffi.string(C.strerror(ffi.errno())))
         return
     end
-    local bytes = #val + 1 -- + LF
-    local nw = C.write(fd, val .. "\n", bytes)
+    local bytes = #val
+    local nw = C.write(fd, val, bytes)
     if nw == -1 then
         logger.err("Cannot write `" .. val .. "` to file `" .. file .. "`:", ffi.string(C.strerror(ffi.errno())))
     end
@@ -930,12 +930,13 @@ function Kobo:getFirmwareVersion()
     local version_str = version_file:read("*line")
     version_file:close()
 
-    local i = 0
-    for field in ffiUtil.gsplit(version_str, ",", false, false) do
-        i = i + 1
-        if (i == 3) then
-             self.firmware_rev = field
+    local i = 1
+    for field in version_str:gmatch("([^,]+)") do
+        if i == 3 then
+            self.firmware_rev = field
+            break
         end
+        i = i + 1
     end
 end
 
@@ -1272,13 +1273,11 @@ function Kobo:toggleChargingLED(toggle)
     --       (when it does, it's an option in the Energy saving settings),
     --       which is why we also limit ourselves to "true" on devices where this was tested.
     -- c.f., drivers/misc/ntx_misc_light.c
-    local f = io.open(self.ntx_lit_sysfs_knob, "we")
-    if not f then
-        logger.err("cannot open", self.ntx_lit_sysfs_knob, "for writing!")
+    local fd = C.open(self.ntx_lit_sysfs_knob, bit.bor(C.O_WRONLY, C.O_CLOEXEC)) -- procfs/sysfs, we shouldn't need O_TRUNC
+    if fd == -1 then
+        logger.err("Cannot open file `" .. self.ntx_lit_sysfs_knob .. "`:", ffi.string(C.strerror(ffi.errno())))
         return false
     end
-    -- Relying on LFs is mildly more elegant than spamming f:flush() calls ;).
-    C.setlinebuf(f)
 
     -- c.f., strace -fittTvyy -e trace=ioctl,file,signal,ipc,desc -s 256 -o /tmp/nickel.log -p $(pidof -s nickel) &
     -- This was observed on a Forma, so I'm mildly hopeful that it's safe on other Mk. 7 devices ;).
@@ -1286,17 +1285,20 @@ function Kobo:toggleChargingLED(toggle)
     if toggle == true then
         -- NOTE: Technically, Nickel forces a toggle off before that, too.
         --       But since we do that on startup, it shouldn't be necessary here...
-        if self.led_uses_channel_3 then
-            f:write("ch 3\n", "cur 1\n", "dc 63\n")
+        for ch = self.led_uses_channel_3 and 3 or 4, 4 do
+            C.write(fd, "ch " .. tostring(ch), 4)
+            C.write(fd, "cur 1", 5)
+            C.write(fd, "dc 63", 5)
         end
-        f:write("ch 4\n", "cur 1\n", "dc 63\n")
     else
         for ch = 3, 5 do
-            f:write("ch " .. tostring(ch) .. "\n", "cur 1\n", "dc 0\n")
+            C.write(fd, "ch " .. tostring(ch), 4)
+            C.write(fd, "cur 1", 5)
+            C.write(fd, "dc 0", 4)
         end
     end
 
-    f:close()
+    C.close(fd)
 end
 
 -- Return the highest core number
@@ -1315,7 +1317,7 @@ end
 
 function Kobo:enableCPUCores(amount)
     -- CPU0 is *always* online ;).
-    for n=1, self.cpu_count do
+    for n = 1, self.cpu_count do
         local path = "/sys/devices/system/cpu/cpu" .. n .. "/online"
         local up
         if n >= amount then
@@ -1324,11 +1326,7 @@ function Kobo:enableCPUCores(amount)
             up = "1"
         end
 
-        local f = io.open(path, "we")
-        if f then
-            f:write(up)
-            f:close()
-        end
+        writeToSys(up, path)
     end
 end
 

frontend/device/sysfs_light.lua

@@ -5,6 +5,10 @@
 local logger = require("logger")
 local dbg = require("dbg")
 
+local ffi = require("ffi")
+local C = ffi.C
+require("ffi/posix_h")
+
 local SysfsLight = {
     frontlight_white = nil,
     frontlight_red = nil,
@@ -131,19 +135,23 @@ function SysfsLight:_set_light_value(sysfs_directory, value)
     self:_write_value(sysfs_directory .. "/brightness", value)
 end
 
-function SysfsLight:_write_value(file, value)
-    local f = io.open(file, "w")
-    if not f then
-        logger.err("Could not open file: ", file)
+function SysfsLight:_write_value(file, val)
+    local fd = C.open(file, bit.bor(C.O_WRONLY, C.O_CLOEXEC)) -- procfs/sysfs, we shouldn't need O_TRUNC
+    if fd == -1 then
+        logger.err("Cannot open file `" .. file .. "`:", ffi.string(C.strerror(ffi.errno())))
         return false
     end
-    local ret, err_msg, err_code = f:write(value)
-    f:close()
-    if not ret then
-        logger.err("Write error: ", err_msg, err_code)
+    val = tostring(val)
+    local bytes = #val
+    local nw = C.write(fd, val, bytes)
+    if nw == -1 then
+        logger.err("Cannot write `" .. val .. "` to file `" .. file .. "`:", ffi.string(C.strerror(ffi.errno())))
+        C.close(fd)
         return false
     end
-    return true
+    C.close(fd)
+    -- NOTE: Allows the caller to possibly handle short writes (not that these should ever happen here).
+    return nw == bytes
 end
 
 return SysfsLight

frontend/ui/uimanager.lua

@@ -556,9 +556,9 @@ function UIManager:setDirty(widget, refreshtype, refreshregion, refreshdither)
         self:_refresh(refreshtype, refreshregion, refreshdither)
         if dbg.is_on then
             if refreshregion then
-                logger.dbg("setDirty", refreshtype, "from widget", widget_name, "w/ region", refreshregion.x, refreshregion.y, refreshregion.w, refreshregion.h, refreshdither and "AND w/ HW dithering" or "")
+                logger.dbg("setDirty", refreshtype, "from widget", widget_name, "w/ region", refreshregion.x, refreshregion.y, refreshregion.w, refreshregion.h, "dithering:", refreshdither)
             else
-                logger.dbg("setDirty", refreshtype, "from widget", widget_name, "w/ NO region", refreshdither and "AND w/ HW dithering" or "")
+                logger.dbg("setDirty", refreshtype, "from widget", widget_name, "w/ NO region; dithering:", refreshdither)
             end
         end
     end
@@ -1085,7 +1085,7 @@ function UIManager:_refresh(mode, region, dither)
     end
 
     -- if we've stopped hitting collisions, enqueue the refresh
-    logger.dbg("_refresh: Enqueued", mode, "update for region", region.x, region.y, region.w, region.h, dither and "w/ HW dithering" or "")
+    logger.dbg("_refresh: Enqueued", mode, "update for region", region.x, region.y, region.w, region.h, "dithering:", dither)
     table.insert(self._refresh_stack, {mode = mode, region = region, dither = dither})
 end
 

frontend/ui/widget/frontlightwidget.lua

@@ -557,15 +557,14 @@ function FrontLightWidget:onTapProgress(arg, ges_ev)
     end
 
     if ges_ev.pos:intersectWith(self.fl_progress.dimen) then
-        -- Unschedule any pending updates.
-        UIManager:unschedule(self.refreshBrightnessWidgets)
-
         local perc = self.fl_progress:getPercentageFromPosition(ges_ev.pos)
         if not perc then
             return true
         end
-        local num = Math.round(perc * self.fl.max)
+        -- Unschedule any pending updates.
+        UIManager:unschedule(self.refreshBrightnessWidgets)
 
+        local num = Math.round(perc * self.fl.max)
         -- Always set the frontlight intensity.
         self:setFrontLightIntensity(num)