Clean up CreateFile loop in ReadUTF8File #15874

Open
opened 2026-01-31 04:51:05 +00:00 by claunia · 1 comment
Owner

Originally created by @DHowett on GitHub (Nov 12, 2021).

ReadUTF8File in the Settings library tries three times to read the settings file. If the size changes while it is being read, it'll try again.

Right now, the code looks like this (roughly):

while(tries) {
    open
    check permissions
    read in full
    check read size
}

It's expensive to open the file and check the permissions every time. Instead, we could:

open
check permissions
while (tries) {
    reset file pointer
    read in full
    check read size
}

That'll save us closing/reopening the file up to three times.

Originally created by @DHowett on GitHub (Nov 12, 2021). `ReadUTF8File` in the Settings library tries three times to read the settings file. If the size changes while it is being read, it'll try again. Right now, the code looks like this (roughly): ``` while(tries) { open check permissions read in full check read size } ``` It's expensive to open the file and check the permissions every time. Instead, we could: ``` open check permissions while (tries) { reset file pointer read in full check read size } ``` That'll save us closing/reopening the file up to three times.
claunia added the Area-SettingsIssue-TaskProduct-TerminalArea-CodeHealth labels 2026-01-31 04:51:06 +00:00
Author
Owner

@DHowett commented on GitHub (Nov 12, 2021):

Though -- what if we fail to open the file because somebody has it locked? Wouldn't we want to retry some time later so as to not frag the user's settings as "invalid"? That suggests that we may want to retry opening it as well...

@DHowett commented on GitHub (Nov 12, 2021): Though -- what if we fail to open the file because somebody has it locked? Wouldn't we want to retry some time later so as to not frag the user's settings as "invalid"? That suggests that we _may_ want to retry opening it as well...
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#15874