[FIX] #995 for Linux and Mac #1049
Conversation
|
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Your PR breaks these cases:
Check the result page for more info. |
| strncpy(path_of_ccextractor, path_of_executable, strlen(path_of_executable)); | ||
| path_of_ccextractor[strlen(path_of_ccextractor)-20]='\0'; // Fails without null pointer | ||
| media.icons.home = icon_load(concat(path_of_ccextractor,"icon/home.png")); | ||
| media.icons.directory = icon_load(concat(path_of_ccextractor, "icon/directory.png")); |
There was a problem hiding this comment.
This is leaking memory - concat is doing a malloc() but there's no free() anywhere, and you're not holding a reference.
A simple solution would be to write a function called cc_icon_load (or a better name you come up with) that just takes the name of the icon ("home") for example and adds everything else (path_of_executable) + ".png", then frees everything, and just returns the same same as icon_load().
A much better solution would be to write a simple program that takes all the icons and generates a .h file that contains the icons themselves (not the filenames; the binary data) in an array so we can generate a binary and contains the icons completely - so we don't have to have any additional file.
There was a problem hiding this comment.
While working on the better solution stbi uses fopen to read the file so unless we rewrite the library for the stbi_load function The better method won't work in the situation
| // nk_draw_text(canvas, space, "Hardsubs Extraction: Yes", 24, &font->handle, nk_rgb(88, 81, 96), nk_rgb(0, 0, 0)); | ||
| //} | ||
|
|
||
| char* get_executable_path(){ |
There was a problem hiding this comment.
This doesn't seem really portable... what about Windows?
There was a problem hiding this comment.
I don't have a Windows Machine to test it on :(
There was a problem hiding this comment.
There is a Method for windows too: https://docs.microsoft.com/en-us/windows/desktop/api/libloaderapi/nf-libloaderapi-getmodulefilenamea but i don't have a machine to test it :) If someone volunteers i'll gladly implement it
| nk_label(ctx, "Hardsubtitles extraction: Yes", NK_TEXT_LEFT); | ||
| else | ||
| nk_label(ctx, "Hardsubtitles extraction: No", NK_TEXT_LEFT); | ||
| nk_label(ctx, concat("Input type: ", input.type[input.type_select]), NK_TEXT_LEFT); |
There was a problem hiding this comment.
Leaks here...
Maybe write a printf() function that works on labels - check the source code of mprintf for example.
You could do a nk_label_printf (ctx, NK_TEXT_LEFT, "Input type: %s%s", input.type[input.type_select]);
which would be a lot neater.
That function in fact could be contributed to nuklear, since it would be quite useful for everybody.
There was a problem hiding this comment.
I didn't do anything here other than convert the mixed identation from spaces and tabs to tabs only. I'll try my best to fix this
| char buf[256]; | ||
| uint32_t size = sizeof(buf); | ||
| if (_NSGetExecutablePath(buf, &size) == 0){ | ||
| return buf; |
There was a problem hiding this comment.
You can't return this buf, it's an stack variable.
3 participants
Please prefix your pull request with one of the following: [FEATURE] [FIX] [IMPROVEMENT].
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
{pull request content here}