Skip to content
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.

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

add -Wextra and remove a lot of warnings for C code #8916

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

uecker
Copy link

@uecker uecker commented Jun 28, 2024

This is for people who need / want the C version of the code.

@stephanecharette
Copy link
Collaborator

What is the difference between this PR and the previous one PR #8915 ?

@uecker
Copy link
Author

uecker commented Jun 29, 2024

None really, the previous one was based on the master branch of my fork and it seems Github still does not allow one to change this for a pull request, so I closed the old one and recreated it using a new branch. In my fork I will go on and remove a lot of things to make a minimalistic C library with just the core functionality.

@stephanecharette
Copy link
Collaborator

I want to reiterate for anyone else who makes it here that the last few years worth of bug fixes, CUDA/cuDNN crash fixes, performance enhancements, etc..., are only in the new Darknet/YOLO repo here: https://github.com/hank-ai/darknet#table-of-contents

@cenit
Copy link
Collaborator

cenit commented Jul 11, 2024

@stephanecharette I already told you to please avoid spamming everywhere your repo, the link in the readme is more than enough. Also because "last few years" is maybe an exaggeration, commits are still made here, but still this repo does not receive all the love it might deserve.
In any case, I thank you @uecker for the contribution, and I'll look forward to merge it soon (after more review needed, of course)

src/box.c Outdated
@@ -152,6 +152,7 @@ float box_iou_kind(box a, box b, IOU_LOSS iou_kind)
{
//IOU, GIOU, MSE, DIOU, CIOU
switch(iou_kind) {
case MSE: assert(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you please align this better with the surrounding lines?

src/classifier.c Outdated
char *input = buff;
while(1){
if(filename){
strncpy(input, filename, 256);
strncpy(input, filename, 255);
Copy link
Collaborator

Choose a reason for hiding this comment

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

sizeof buff -1


ifeq ($(USE_CPP), 1)
# C is not C++
CFLAGS+=-fpermissive -Wno-write-strings
Copy link
Collaborator

Choose a reason for hiding this comment

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

fpermissive really necessary?

Copy link
Author

Choose a reason for hiding this comment

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

I guess one could avoid it, but then one can not write idiomatic C. I would recommend to remove the USE_CPP flag anyway. What is the benefit? That C++ does better type checking is not true anymore for a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants