-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issues with setting dictionaries as read-only #6
Comments
This seems to be a firmware issue (may not be specific to Windows). The following patch can be used as a temporary workaround for now: # kobopatch v0.15.0, firmware 4.20.14601+ (last tested on 4.20.14622)
Never sync dictionaries:
- Enabled: no
- BaseAddress: {Sym: "SyncDictionariesCommand::prepareDownloadList()"}
- ReplaceBytes: {Offset: 1048, FindH: 0CD5, ReplaceH: 0CE0} #permissions
- ReplaceBytes: {Offset: 1026, FindH: FFF68DAE, ReplaceInstNOP: true} #size
- ReplaceBytes: {Offset: 992, FindH: 3FF49EAE, ReplaceInstNOP: true} #isSynced # firmware 4.20.14622
Never sync dictionaries:
- Enabled: no
- ReplaceBytes: {Offset: 0x6108B0, FindH: 0CD5, ReplaceH: 0CE0} #permissions
- ReplaceBytesNOP: {Offset: 0x61089A, FindH: FFF68DAE} #size
- ReplaceBytesNOP: {Offset: 0x610878, FindH: 3FF49EAE} #isSynced The issue seems to be that under some (TBD) conditions, the permissions check never gets reached, presumably due to the size or isSynced check bypassing it. It was first seen on firmware 4.20.14622, but could have existed as early as 4.20.14601 (but I never noticed it myself). I'll be looking into this further later. So far, I've confirmed this bug with two Windows users, both when setting it read-only through dictutil and manually. The write bit isn't present when checking over telnet, and a trace of libnickel shows that it was seen, but the logs don't show it being skipped. |
On a slightly unrelated note, @davidfor pointed out how this read-only functionality has been around for a while, but we never ended up noticing it before my work in #3 (prior to the recent firmware versions, the dicthtml-micthtml patch prevented overwriting overridden dictionaries, and before #3, I found that IsSynced in the DB could be used to skip the sync process, but that was obsoleted by the removal of the table in 14622). This read-only functionality seems to have been present as early 11655. |
@gtalusan, is this a bug in the firmware (the size or issynced check preventing the permission check from actually having an effect)? Also, another bug which is more visible (it's cosmetic only), but just as simple: sideloaded dictionaries always appear as pending. |
Some stuff I've been looking into on 14622:
cc @davidfor |
This is to debug the read-only dictionary replacement bug in 14622. See pgaskin/dictutil#6 (comment).
Yay! I've So, I have multiple dictionaries installed:
I then installed my updated version of dictbug-trace, and ran a sync with the
I'm going to check to make sure I'm correct after this. |
For comparison, here's the logs for the automatic sync after installing the Italian dictionary from the settings:
And a sync after it's been installed:
So, I'll need to figure out what conditions the file size check with |
Using this, I've narrowed down further. It's not related to |
This is an educated guess (I'm still looking at it), but I think the issue can be fixed by swapping the order of the size/permission checks so permissions come first. EDIT: Even after my discovery below, this would likely still fix it, but it may not be the root cause of the bug. |
An update for the comment a few ones above (I missed the FileSizeGetter stuff, which is probably why some parts of this bug didn't make complete sense to me after I eliminated
This means the fact that the size check patch worked was likely a coincidence, and the real issue is either there or in the permissions check. I'm going to need to add some hooks at each of the checks (which show the result and value) to figure this out. Since this is a bit more work to do, and the bug seems relatively straightforward overall, I'll wait and see if this is fixed in the next firmware version (if there's one within the next week or two) before looking at this further. |
@geek1011 I copied your patch to src/nickel.yaml, but it doesn't work. I tried to set dictutil v0.3.1 |
You need to put it into libnickel, not nickel. |
@geek1011 Thank you, also I missed this: Never sync dictionaries:
- Enabled: no
- BaseAddress: {Sym: "SyncDictionariesCommand::prepareDownloadList()"}
- ReplaceBytes: {Offset: 922, FindH: 0CD5, ReplaceH: 0CE0} #permissions
- ReplaceBytes: {Offset: 900, FindH: FFF6CAAE, ReplaceInstNOP: true} #size
- ReplaceBytes: {Offset: 866, FindH: 3FF4DBAE, ReplaceInstNOP: true} #isSynced |
@geek1011 Thanks a lot for your work. It seems with the latest firmware, regardless of the read only flag, the device will still replace custom dicts. Right now, am using the custom (translation) dicts + 'Enable searches on extra dictionaries patch'. Which means firmware patching is still needed. Cheers. |
This issue is still there in 15268. |
Same with 15505. |
No description provided.
The text was updated successfully, but these errors were encountered: