Page MenuHomeFreeBSD

libc: Fix a few bugs in the xlocale collation code.
ClosedPublic

Authored by markj on Jan 9 2020, 4:42 PM.
Tags
None
Referenced Files
F131637181: D23109.id66576.diff
Thu, Oct 9, 10:52 PM
Unknown Object (File)
Fri, Oct 3, 12:24 AM
Unknown Object (File)
Sat, Sep 27, 1:08 PM
Unknown Object (File)
Sat, Sep 27, 12:07 AM
Unknown Object (File)
Sat, Sep 13, 1:04 AM
Unknown Object (File)
Fri, Sep 12, 5:38 PM
Unknown Object (File)
Aug 20 2025, 8:40 AM
Unknown Object (File)
Aug 4 2025, 5:27 AM
Subscribers

Details

Summary
  • Fix checks for mmap() failures.
  • Set the "map" and "maplen" fields of struct xlocale_collate so that the destructor actually works.
  • Free an already-mapped collation file before loading a new one into the global table.
  • Harmonize the prototype and definition of __collate_load_tables_l() by adding the "static" qualifier to the latter.

PR: 243195

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 28574
Build 26618: arc lint + arc unit

Event Timeline

markj added reviewers: bapt, yuripv, cem.

Defer the unmap until we are sure that __collate_load_tables_l() won't fail.

I wonder if we could instead check if we have the table already loaded and return without doing everything over and over again. Something like the following right at the start:

if (table->__collate_load_error == 0 && table->header != NULL && strcmp(table->header.locale, encoding) == 0)
    return (_LDP_LOADED);

Not completely sure about the table->header check here as we already checked that the table was loaded, but it's just an idea.

(P.S. I think encoding is really a wrong argument name here, as it's full locale name.)

I wonder if we could instead check if we have the table already loaded and return without doing everything over and over again. Something like the following right at the start:

if (table->__collate_load_error == 0 && table->header != NULL && strcmp(table->header.locale, encoding) == 0)
    return (_LDP_LOADED);

Not completely sure about the table->header check here as we already checked that the table was loaded, but it's just an idea.

(P.S. I think encoding is really a wrong argument name here, as it's full locale name.)

Nevermind me, I obviously can't read several lines up :)

This revision is now accepted and ready to land.Jan 9 2020, 5:43 PM

I wonder if we could instead check if we have the table already loaded and return without doing everything over and over again. Something like the following right at the start:

if (table->__collate_load_error == 0 && table->header != NULL && strcmp(table->header.locale, encoding) == 0)
    return (_LDP_LOADED);

Not completely sure about the table->header check here as we already checked that the table was loaded, but it's just an idea.

(P.S. I think encoding is really a wrong argument name here, as it's full locale name.)

Nevermind me, I obviously can't read several lines up :)

Hmm, I don't see what lines you are referring to. Indeed, this function does not handle the case where a table is already loaded. I'd prefer to fix that separately though.

This revision was automatically updated to reflect the committed changes.