From d9eb6e971742810bb1ee545f80686a84b90c06d3 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Fri, 14 Oct 2022 01:50:03 +0200 Subject: [PATCH] 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 | 2 +- frontend/device/kobo/device.lua | 44 ++++++++++++------------- frontend/device/sysfs_light.lua | 26 ++++++++++----- frontend/ui/uimanager.lua | 6 ++-- frontend/ui/widget/frontlightwidget.lua | 7 ++-- 5 files changed, 45 insertions(+), 40 deletions(-) diff --git a/base b/base index 4216c40d6..37506f4d5 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 4216c40d662660c9bbc48e79893b437e97518254 +Subproject commit 37506f4d552e9ccf2f6c0f71afe925c3ffd9abd2 diff --git a/frontend/device/kobo/device.lua b/frontend/device/kobo/device.lua index 1166ce39a..b6fbdeac7 100644 --- a/frontend/device/kobo/device.lua +++ b/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 diff --git a/frontend/device/sysfs_light.lua b/frontend/device/sysfs_light.lua index b30231531..78d3653c9 100644 --- a/frontend/device/sysfs_light.lua +++ b/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 diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 325dda358..afd1ccb15 100644 --- a/frontend/ui/uimanager.lua +++ b/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 diff --git a/frontend/ui/widget/frontlightwidget.lua b/frontend/ui/widget/frontlightwidget.lua index 0ee004138..0ef5b8f38 100644 --- a/frontend/ui/widget/frontlightwidget.lua +++ b/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)