ravachol/kew
13
189
Fork
You've already forked kew
31

simplify macro and replace fprintf with perror for memory allocation #208

Open
drex_vk wants to merge 2 commits from drex_vk/kew:simplify-macro-and-add-perror into main
pull from: drex_vk/kew:simplify-macro-and-add-perror
merge into: ravachol:main
ravachol:main
ravachol:develop
ravachol:artistsdb
First-time contributor
Copy link

Hello.

i replace #ifdef with #if defined for upgrade the lisibility and for the futures (for write two conditions) and i replace fprintf with perror for memory-allocation because is a operation of OS but perror give a more accurate error message.

Hello. i replace #ifdef with #if defined for upgrade the lisibility and for the futures (for write two conditions) and i replace fprintf with perror for memory-allocation because is a operation of OS but perror give a more accurate error message.
simplify macro and replace fprintf with perror for memory allocation
Some checks failed
macOS Latest Build Check / macOS Build Check (pull_request) Has been cancelled
Ubuntu Build Check / Ubuntu Build Check (pull_request) Has been cancelled
macOS x86_64 Build Check / Build on Intel macOS (pull_request) Has been cancelled
d4e14e219e

Hi,

Thank you for doing this. I think I prefer the short #ifdef, unless there's some advantage to #if define other than readability.

As for perror, this should probably be done, but if we are going to do it, I think we should do it in every place where it is preferred, instead of being inconsistent.

Hi, Thank you for doing this. I think I prefer the short #ifdef, unless there's some advantage to #if define other than readability. As for perror, this should probably be done, but if we are going to do it, I think we should do it in every place where it is preferred, instead of being inconsistent.
Author
First-time contributor
Copy link

Hello thanks you for your feedback so for #if defined is better if you want to write two in condition **example #if defined(_WIN32) || defined(_WIN64) while #ifdef is not possible.

drex_vk :)

Hello thanks you for your feedback so for #if defined is better if you want to write two in condition **example #if defined(_WIN32) || defined(_WIN64) while #ifdef is not possible. drex_vk :)
Author
First-time contributor
Copy link

and also I'm going to put the perror not worried it was just because I wanted to know if it was valid

and also I'm going to put the perror not worried it was just because I wanted to know if it was valid

ok, keep let's keep the #ifdef style in that case

ok, keep let's keep the #ifdef style in that case
Author
First-time contributor
Copy link

okay no problem

okay no problem
update commit
Some checks are pending
macOS Latest Build Check / macOS Build Check (pull_request) Blocked by required conditions
Ubuntu Build Check / Ubuntu Build Check (pull_request) Blocked by required conditions
macOS x86_64 Build Check / Build on Intel macOS (pull_request) Blocked by required conditions
7e6abaac43
Author
First-time contributor
Copy link

hi it's good normally I put perror everywhere I could and i remove #if defined() where I modified the #ifdef.

so your project is so very stylish and you code in objective-C ?

drex_vk :)

hi it's good normally I put perror everywhere I could and i remove #if defined() where I modified the #ifdef. so your project is so very stylish and you code in objective-C ? drex_vk :)
Author
First-time contributor
Copy link

And look carefully just in case because sometimes I make mistakes inadvertently

And look carefully just in case because sometimes I make mistakes inadvertently

No, I don't code in objective-c, but there is a little bit of objective-c and it's for the macOS media keys, which can't be done any other way. But I didn't do that one.

What I meant regarding perror is to replace fprintf in all places in /src where perror would be better, instead of having two ways, inconsistently.

No, I don't code in objective-c, but there is a little bit of objective-c and it's for the macOS media keys, which can't be done any other way. But I didn't do that one. What I meant regarding perror is to replace fprintf in all places in /src where perror would be better, instead of having two ways, inconsistently.
Author
First-time contributor
Copy link

Hi
Okay

Hi Okay
Author
First-time contributor
Copy link

so okay but what's missing in the code because I put perror where I could instead of fprintf or printf but it depends because perror is only used in OS operations

so okay but what's missing in the code because I put perror where I could instead of fprintf or printf but it depends because perror is only used in OS operations

@drex_vk wrote in #208 (comment):

so okay but what's missing in the code because I put perror where I could instead of fprintf or printf but it depends because perror is only used in OS operations

There are a lot more places that use malloc for instance than those files.

@drex_vk wrote in https://codeberg.org/ravachol/kew/pulls/208#issuecomment-18396215: > so okay but what's missing in the code because I put perror where I could instead of fprintf or printf but it depends because perror is only used in OS operations There are a lot more places that use malloc for instance than those files.
Author
First-time contributor
Copy link

okay i will fix this i do it thanks you for the feedback

okay i will fix this i do it thanks you for the feedback

Just search for fprintf. It's not that many that are relevant. Ignore miniaudio.h and others in /include directory.

Just search for fprintf. It's not that many that are relevant. Ignore miniaudio.h and others in /include directory.
Author
First-time contributor
Copy link

okay

okay
Some checks are pending
macOS Latest Build Check / macOS Build Check (pull_request) Blocked by required conditions
Ubuntu Build Check / Ubuntu Build Check (pull_request) Blocked by required conditions
macOS x86_64 Build Check / Build on Intel macOS (pull_request) Blocked by required conditions
This pull request has changes conflicting with the target branch.
  • src/common/appstate.c
Some workflows are waiting to be reviewed.
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u simplify-macro-and-add-perror:drex_vk-simplify-macro-and-add-perror
git switch drex_vk-simplify-macro-and-add-perror

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff drex_vk-simplify-macro-and-add-perror
git switch drex_vk-simplify-macro-and-add-perror
git rebase main
git switch main
git merge --ff-only drex_vk-simplify-macro-and-add-perror
git switch drex_vk-simplify-macro-and-add-perror
git rebase main
git switch main
git merge --no-ff drex_vk-simplify-macro-and-add-perror
git switch main
git merge --squash drex_vk-simplify-macro-and-add-perror
git switch main
git merge --ff-only drex_vk-simplify-macro-and-add-perror
git switch main
git merge drex_vk-simplify-macro-and-add-perror
git push origin main
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ravachol/kew!208
Reference in a new issue
ravachol/kew
No description provided.
Delete branch "drex_vk/kew:simplify-macro-and-add-perror"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?