8000 Fix nesting of free(). by mserajnik · Pull Request #2601 · vmangos/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix nesting of free(). #2601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 30, 2024
Merged

Fix nesting of free(). #2601

merged 1 commit into from
Apr 30, 2024

Conversation

mserajnik
Copy link
Contributor
@mserajnik mserajnik commented Apr 30, 2024 •
edited
Loading

🍰 Pullrequest

This fixes the GCC/Clang build error:

/home/runner/work/core/core/src/mangosd/CliRunnable.cpp: In member function ‘void CliRunnable::operator()()’:
/home/runner/work/core/core/src/mangosd/CliRunnable.cpp:128:14: error: ‘command_str’ was not declared in this scope
  128 |         free(command_str);
      |              ^~~~~~~~~~~
[ 99%] Building CXX object src/mangosd/CMakeFiles/mangosd.dir/MaNGOSsoap.cpp.o
make[2]: *** [src/mangosd/CMakeFiles/mangosd.dir/build.make:76: src/mangosd/CMakeFiles/mangosd.dir/CliRunnable.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:556: src/mangosd/CMakeFiles/mangosd.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

The new if (s_canReadLine) introduced in 79c5f50 probably caused the auto-merge to fuck up the nesting.

The PR also makes it so free() is used on Windows as well; when #2572 was opened, readline support didn't exist on Windows, but now it does (so the free() needs to happen there as well).

Proof

  • None

Issues

  • None

How2Test

  • None

Todo / Checklist

  • None

@mserajnik
Copy link
Contributor Author

But I just realized, since 79c5f50 introduced readline support for Windows, the free() should probably happen on Windows as well?

@imranh2
Copy link
Contributor
imranh2 commented Apr 30, 2024

Yep, we should do an unconditional free, just remove the preprocessor ifdefs

@mserajnik
Copy link
Contributor Author

Yep, we should do an unconditional free, just remove the preprocessor ifdefs

Yeah, I didn't realize this at first because I was only focused on the wrong nesting. 😄
I adjusted this PR now.

mserajnik referenced this pull request Apr 30, 2024
@imranh2
Fix linux daemon issue (#2572)
657d4f5 
* [linux][mangosd] fix a memory leak in console CLI

* [mangosd][daemon] Don't start CLI when launched in daemon mode

* [linux][daemon] don't change dirs -> don't break relative paths
@ratkosrb ratkosrb merged commit 50b8a58 into vmangos:development Apr 30, 2024
3 checks passed
ratkosrb added a commit that referenced this pull request Jun 21, 2024
@ratkosrb
Revert "Fix nesting of free. (#2601)"
17cc7bc 
This reverts commit 50b8a58.
@mserajnik mserajnik deleted the fix-build-error branch September 26, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mserajnik @imranh2 @ratkosrb
0