For more consistent usage of the Windows API. `_wgetcwd` seems to be a wrapper around `GetCurrentDirectoryW` anyway, at least in the [Wine API](https://source.winehq.org/ident?_i=_wgetcwd).
Pass the correct size of the wide char buffer instead of passing the size of the output parameter buffer.
Remove a nullptr check. The argument should never be null and we don't check for null arguments anywhere else.
For more consistent usage of the Windows API. `_wchdir` seems to be a wrapper around `SetCurrentDirectoryW` anyway, at least in the [Wine API](https://source.winehq.org/ident?_i=_wchdir).
For more consistent usage of the Windows API. `_wmkdir` seems to be a wrapper around `CreateDirectoryW` anyway, at least in the [Wine API](https://source.winehq.org/ident?_i=_wmkdir).
Previously, the socket addresses were truncated as the `msg_namelen`
field is both input **and** output: After receiving an IPv4 packet, the
socket address field would be too short for an IPv6 address.
Remove square brackets to reduce the amount of space used.
Add log level indicator. The position between timestamp and system was
chosen because it is at a fixed position (unlike after the system) but
the log still remains naively sortable (which wouldn't happen if we were
to place it in front of the timestamp.
Before:
```
[2022-04-29 15:25:37][engine]: running on unix-linux-amd64
[2022-04-29 15:25:37][engine]: arch is little endian
[2022-04-29 15:25:37][storage]: added path '$USERDIR' ('/path/to/home/.teeworlds')
[2022-04-29 15:25:37][storage]: added path '$DATADIR' ('data')
[2022-04-29 15:25:37][storage]: added path '$CURRENTDIR' ('/path/to/ddnet')
[2022-04-29 15:25:37][host_lookup]: host='localhost' port=0 1
[2022-04-29 15:25:37][host_lookup]: host='localhost' port=0 2
[2022-04-29 15:25:37][console]: executing 'autoexec_server.cfg'
```
After:
```
2022-04-29 15:25:37 I engine: running on unix-linux-amd64
2022-04-29 15:25:37 I engine: arch is little endian
2022-04-29 15:25:37 I storage: added path '$USERDIR' ('/path/to/home/.teeworlds')
2022-04-29 15:25:37 I storage: added path '$DATADIR' ('data')
2022-04-29 15:25:37 I storage: added path '$CURRENTDIR' ('/path/to/ddnet')
2022-04-29 15:25:37 I host_lookup: host='localhost' port=0 1
2022-04-29 15:25:37 I host_lookup: host='localhost' port=0 2
2022-04-29 15:25:37 I console: executing 'autoexec_server.cfg'
```
5760: Fix variable shadow warnings with MinGW r=def- a=Robyt3
```
ddnet/src/base/system.cpp: In function 'int byteval(const char*, unsigned char*)':
ddnet/src/base/system.cpp:3044:32: warning: declaration of 'byte' shadows a global declaration [-Wshadow]
3044 | static int byteval(const char *byte, unsigned char *dst)
| ~~~~~~~~~~~~^~~~
In file included from msys64/mingw64/include/objbase.h:8,
from ddnet/src/base/system.cpp:68:
msys64/mingw64/include/rpcndr.h:63:25: note: shadowed declaration is here
63 | typedef unsigned char byte;
| ^~~~
ddnet/src/base/system.cpp: In function 'unsigned char str_byte_next(const char**)':
ddnet/src/base/system.cpp:3557:23: warning: declaration of 'byte' shadows a global declaration [-Wshadow]
3557 | unsigned char byte = **ptr;
| ^~~~
msys64/mingw64/include/rpcndr.h:63:25: note: shadowed declaration is here
63 | typedef unsigned char byte;
| ^~~~
ddnet/src/base/system.cpp: In function 'int str_utf8_decode(const char**)':
ddnet/src/base/system.cpp:3577:31: warning: declaration of 'byte' shadows a global declaration [-Wshadow]
3577 | unsigned char byte = str_byte_next(ptr);
| ^~~~
msys64/mingw64/include/rpcndr.h:63:25: note: shadowed declaration is here
63 | typedef unsigned char byte;
| ^~~~
```
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5754: Auto refresh skins when changing related settings r=def- a=Jupeyy
motivation: downloaded skins aren't resettet, e.g. if they failed before. config change -> resets everything
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
5757: Make file link absolute, add `fs_is_relative_path` r=def- a=Robyt3
This fixes links not opening for relative paths, as links like `file://temp/skins` cannot be resolved by the shell.
Closes#5746.
## Checklist
- [X] Tested the change ingame (only on Windows)
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
Co-authored-by: Robert Müller <robytemueller@gmail.com>
On Windows, the check was incorrectly using logical or instead of bitwise or. Checking for the flag `FILE_ATTRIBUTE_REPARSE_POINT` is not necessary, as the `FILE_ATTRIBUTE_DIRECTORY` will correctly be set when the target of a symbolic link is a directory. This otherwise causes symbolic links to files to be incorrectly handled as directories.
On Linux, the minor performance optimization of using `entry->d_type` is reverted and `fs_is_dir` is used instead. This internally uses `stat`, which correctly returns the attributes for the symbolic link targets.
5205: Allow multiple addresses per server in the serverbrowser r=def- a=heinrich5991
Support is incomplete for `leak_ip_address_to_all_servers` (will only
ping the first address of each server) and for the `leak_ip` setting
(which will also only ping the first address of each server).
Fixes#5158.
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
Support is incomplete for `leak_ip_address_to_all_servers` (will only
ping the first address of each server) and for the `leak_ip` setting
(which will also only ping the first address of each server).
5504: Remove useless typedef in c++ r=Jupeyy a=Chairn
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Chairn <chairn.nq@hotmail.fr>
- `io_read_all` reads all bytes from a file handle into a new buffer. It should only need one allocation per file in cases where the actual file size matches the expected file size. Otherwise it falls back to doubling the buffer size if the actual file size is larger than expected to avoid TOCTOU problems.
- `io_read_all_str` reads all bytes from a file handle into a new buffer and also ensures that the buffer is null-terminated and contains no other null-characters.
- `mem_has_null` is a utility used by `io_read_all_str` to ensure that no null-characters exist in the bytes read from the file.
5406: Remove namespace tw r=Jupeyy a=heinrich5991
It didn't have a clear role, it just acted as a distinguisher between
two functions with the same name.
Rename `tw::time_get` to `time_get_nanoseconds` and delete the old
`time_get_nanoseconds`. Move `CCmdlineFix` and the typed
`net_socket_read_wait` function to the global namespace.
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
It didn't have a clear role, it just acted as a distinguisher between
two functions with the same name.
Rename `tw::time_get` to `time_get_nanoseconds` and delete the old
`time_get_nanoseconds`. Move `CCmdlineFix` and the typed
`net_socket_read_wait` function to the global namespace.
5351: Refactoring: Use MultiByteToWideChar instead of str_utf16le_encode r=heinrich5991 a=Robyt3
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5332: Pass output parameter to WriteFile that is required on Windows 7 r=heinrich5991 a=Robyt3
Maybe closes#5322. I can't reproduce the crash. Can you try if this fixes it, `@Chairn?`
Looks like Windows 7 needs this parameter.
Alternative to #5331.
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5327: Add CLockScope r=heinrich5991 a=def-
Remove unused lock and scope in threading.h
Not sure if this is preferred
We could also try switching to std::mutex and lock_guard
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Dennis Felsing <dennis@felsin9.de>
5260: Pr thread safety negative r=heinrich5991 a=def-
WorkerThread is hard because `REQUIRES(!((CJobPool *)pUser)->m_Lock)` would require alias analysis or the function using that everywhere.
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Dennis Felsing <dennis@felsin9.de>
5291: Windows: Use current console background color, restore original color on exit, refactoring r=heinrich5991 a=Robyt3
- Remember initial console foreground and background color.
- Use the original background color with the message specific foreground color, so it looks better when the console background color isn't black.
- Restore the initial console foreground and background color on exit instead of leaving the color be whatever was last used.
- Add locks to ensure `Log`-calls from different threads don't overlap their output/color.
- Refactoring: Replace numbers with color constants defined in `wincon.h` (4312d20923/include/wincon.h (L51-L67)).
- Refactoring: Slightly reorder the `if`s and remove redundant checks which are ensured by the `else-if` structure.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5179: Add loglevel config (fixes#5178, fixes#5174) r=heinrich5991 a=def-
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
5272: Handle failed socket creation (fixes#5267) r=heinrich5991 a=def-
by returning nullpointer, as the calling functions expect
Broken in 471bb441a1
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
5264: Prevent users from setting port 1-1023 r=Jupeyy a=def-
Following user report that they set cl_port 1 and it didn't work
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
5266: Use std::make_unique (fixes#5166) r=Jupeyy a=def-
No idea why clang-tidy's modernize-... didn't work
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
5252: Use std::vector instead of array, remove base/tl/array.h, algorithm.h, allocator.h, range.h r=def- a=Robyt3
Replaces all usages of `array` with `std::vector`.
I adjusted variable names of variables I changed to use the `v` prefix. Not so in the editor however, as there are already many changes due to clang-tidy enforcing the use of for-each loops.
This allows us to remove all remaining `base/tl` headers except `threading.h`.
Clang-tidy now finds `clang-analyzer-cplusplus.NewDelete` (Use of memory after it is freed), which is also fixed here, though it appears to be a false-positive.
This last remaining usages of `goto` are also removed.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5246: Use std::vector instead of sorted_array, remove base/tl/sorted_array.h r=def- a=heinrich5991
This replaces all usages of `sorted_array` with `std::vector`. This requires the following general changes:
- `add_unsorted` is replaced with `push_back`.
- `add` is replaced with `push_back` and subsequent `sort` or `stable_sort` must be ensured.
- In some cases, immediately sorting the entire list after adding an item was unavoidable. Previously items were added at the correct position, which was O(N) because all items after the inserted one had to be moved in any case.
- `sort_range` is replaced with `sort` or `stable_sort`.
- `size` returns a `size_t` instead of `int`, so to fix sign comparison warnings, casts are added where necessary or types of loop variables are changed to `size_t` where possible. For-each loops are also used where possible / where required by clang-tidy.
- `find_binary` is replaced with `std::equal_range`. This can only find items of the same type, so some wrappers, that only have the relevant fields set, need to be created for searching.
In terms of behavior, this should not change anything, except maybe `CLocalizationDatabase` for the better. As far as I understand it, at lot of the code there was not doing anything. It assumes that binary search can return a range of multiple entries, but the equality/comparison function is based on hash and context hash. This means that any item in this range will match the given hash and context hash already, so all of the following checks are redundant. I changed this to first do a lookup with the hash and context hash and if that fails do another lookup with the default context hash.
I have also already replaced `base/tl/array.h` with `std::vector`, removing all of `base/tl` except `threading.h`. I'll make a separate PR later because this caused a lot more changes especially in the editor that I first want to test and review myself.
Naming of `array`/`sorted_array`/`std::vector` variables was rather inconsistent (sometimes prefix `a` or `l` is used), so ~~I chose to not use any prefix for all new `std::vector`s~~ heinrich5991 left them as-is.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
Because no tools can detect it uninitialized well, tried Memory
Sanitizer (needs all libs including libc++ compiled with it), valgrind
(only detects in LTO build), compiler warnings. Might be related to
unions.
5072: Updated unicode script generation (fixes https://github.com/ddnet/ddnet/issues/5017) r=def- a=Chairn
I also updated the version to 15.0.0. However, confusables are still from version 14 as https://www.unicode.org/Public/security/15.0.0/ is empty.
CC `@heinrich5991`
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Chairn <chairn.nq@hotmail.fr>
This makes it almost as short to log as with `dbg_msg` (one to two more
characters), and much shorter than
`Console()->Print(IConsole::OUTPUT_LEVEL_{STANDARD,ADDINFO,DEBUG})`.
