Skip to content

Commit

Permalink
set locale to C to fix buggy dkjson version closes #3
Browse files Browse the repository at this point in the history
  • Loading branch information
@mjasny
mjasny committed Feb 25, 2024
1 parent 431dc35 commit c20b45f
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions youtube-dl.lua
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
-- fix bug with old buggy dkjson version, see issue #3
os.setlocale("C")

JSON = require "dkjson" -- load additional json routines

-- Probe function.
Expand Down

4 comments on commit c20b45f

@jtojnar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be wary. setlocale(3) is not thread-safe and, if this is just a wrapper around the libc function, there is a chance of it seriously fucking up VLC.

@mjasny
Copy link
Owner Author

@mjasny mjasny commented on c20b45f Feb 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, maybe it is better to leave it there as a comment

@jtojnar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do that without serious warning. In worst case, people might get random crashes – much harder to debug than explicit errors.

If you really want to fix it, just ask people to download latest version of dkjson until https://code.videolan.org/videolan/vlc/-/merge_requests/3318 is merged.

@mjasny
Copy link
Owner Author

@mjasny mjasny commented on c20b45f Feb 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for being so critical, I removed the whole commits to avoid any confusion.

Please # to comment.