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

Minor style-related improvements are made. #68

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

Conversation

varikvalefor
Copy link

No description provided.

…RNING: This thing is untested and may need some TLC.
@eklhad
Copy link
Collaborator

eklhad commented Aug 26, 2021 via email

@varikvalefor
Copy link
Author

varikvalefor commented Aug 27, 2021

The switch you wrote in main.c, yes it needs some tlc;
you replaced my continue statements with breaks.
That changes the flow of the code.
If you left them as continue I think it would work.

VARIK attempted to test the commit in question; if this attempt had succeeded, then this problem probably would have been fixed. However, the C compiler claims that src/url.c is not found, which is correct.

The switch you put into buffers.c, switch(ftype) {
there are no break statements on the cases, so they all fall through to the last assignment.
That won't work.

A fine point is made. Maybe C should learn from Haskell. ;^)

Did you mean
for(; *s; ++s)

A typo is revealed! Luckily, the quoted interpretation is correct. Thanks for asking.

By the way, on a similar but relatively rewrite-intensive note, a decent bit of repetition is found within the source code. Some functions are also very large, occasionally being dozens if not hundreds of lines long, and cannot be understood particularly easily. Consider breaking these functions into relatively small functions.

@eklhad
Copy link
Collaborator

eklhad commented Aug 27, 2021 via email

@varikvalefor
Copy link
Author

This is strange to me. Do you have the latest? Some time ago we merged several small sourcefiles into isup.c, so yes url.c is gone, but the latest makefile should not reference it either. git status - do you have a stale makefile or Makefile or GNUmakefile or such lying around? My makefile doesn't reference url.c at all. Karl Dahlke

The cmake stuff is broken. As of the creation of this comment, the standard makeworks fine.

@eklhad
Copy link
Collaborator

eklhad commented Aug 27, 2021 via email

@varikvalefor
Copy link
Author

Yes, sorry.

Stop apologising for doing God's work. cmake is crap. ;^)

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.

2 participants