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.

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

meta: add clang-format and reformat managarm sysdeps #1176

Merged
merged 5 commits into from
Nov 30, 2024
Merged

meta: add clang-format and reformat managarm sysdeps #1176

merged 5 commits into from
Nov 30, 2024

Conversation

64
Copy link
Member

@64 64 commented Oct 24, 2024

No description provided.

@64 64 changed the title meta: Add clang-format meta: add clang-format Oct 24, 2024
Geertiebear
Geertiebear reviewed Oct 24, 2024
View reviewed changes
.clang-format Outdated Show resolved Hide resolved
@64 64 force-pushed the clang-format branch 5 times, most recently from 6f4ad0e to e6f7be2 Compare October 26, 2024 00:07
avdgrinten
avdgrinten requested changes Oct 26, 2024
View reviewed changes
Copy link
Member

@avdgrinten avdgrinten left a comment

Choose a reason for hiding this comment

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

Looks good on a high level but some aspects definitely need fixing. I marked the major issues with ❌.

options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
@64 64 force-pushed the clang-format branch 2 times, most recently from e98076e to b6dbe00 Compare October 27, 2024 20:38
Dennisbonke
Dennisbonke reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@Dennisbonke Dennisbonke left a comment

Choose a reason for hiding this comment

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

Some notes

.clang-format Outdated Show resolved Hide resolved
.clang-format Outdated Show resolved Hide resolved
options/rtld/generic/linker.cpp Outdated
@@ -643,7 +703,7 @@ void ObjectRepository::_parseDynamic(SharedObject *object) {
object->stringTableOffset = dynamic->d_un.d_ptr;
break;
case DT_STRSZ:
break; // we don't need the size of the string table
break; // we don't need the size of the string table
Copy link
Member

Choose a reason for hiding this comment

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

This seems weird, why is there an extra space here?

options/rtld/generic/linker.cpp Outdated
case DT_RELA: case DT_RELASZ: case DT_RELAENT: case DT_RELACOUNT:
case DT_REL: case DT_RELSZ: case DT_RELENT: case DT_RELCOUNT:
case DT_RELR: case DT_RELRSZ: case DT_RELRENT:
case DT_NEEDED: // we handle this later
Copy link
Member

Choose a reason for hiding this comment

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

Double space here too before we start the comment.

options/rtld/generic/linker.cpp Outdated Show resolved Hide resolved
@64 64 force-pushed the clang-format branch 12 times, most recently from 67dfec2 to 9fc3e84 Compare November 4, 2024 19:52
@64 64 marked this pull request as ready for review November 4, 2024 19:52
@64 64 force-pushed the clang-format branch 5 times, most recently from 1afd181 to 3bc3270 Compare November 8, 2024 01:36
64
64 commented Nov 13, 2024
View reviewed changes
options/ansi/generic/stdio.cpp Outdated Show resolved Hide resolved
options/ansi/generic/stdio.cpp Outdated Show resolved Hide resolved
options/ansi/generic/stdio.cpp Outdated
int fwide(FILE *, int) MLIBC_STUB_BODY wint_t getwc(FILE *) MLIBC_STUB_BODY wint_t
getwchar(void) MLIBC_STUB_BODY wint_t putwc(wchar_t, FILE *) MLIBC_STUB_BODY wint_t
putwchar(wchar_t)
MLIBC_STUB_BODY
Copy link
Member Author

Choose a reason for hiding this comment

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

and here

options/ansi/generic/string.cpp Outdated
case ENOTSUP: s = "Operation not supported (ENOTSUP)"; break;
case ENOTTY: s = "Inappropriate ioctl for device (ENOTTY)"; break;
case EOVERFLOW: s = "Value too large for defined datatype (EOVERFLOW)"; break;
switch (e) {
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe disable here

options/ansi/include/mlibc/file-io.hpp Outdated Show resolved Hide resolved
options/ansi/include/stdio.h Outdated
void setbuf(FILE *__restrict __stream, char *__restrict __buffer);
int setvbuf(FILE *__restrict __stream, char *__restrict __buffer, int __mode, size_t __size);
void setlinebuf(FILE *__stream);
void setbuffer(FILE *__stream, char *__buffer, size_t __size);

/* [C11-7.21.6] Formatted input/output functions */

__attribute__((__format__(__printf__, 2, 3)))
int fprintf(FILE *__restrict __stream, const char *__restrict __format, ...);
__attribute__((__format__(__printf__, 2, 3))) int
Copy link
Member Author

Choose a reason for hiding this comment

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

yikes

options/glibc/include/err.h Outdated
__attribute__((__noreturn__, __format__(__printf__, 2, 0)))
void verrx(int __errnum, const char *__format, va_list __args);
__attribute__((__noreturn__, __format__(__printf__, 2, 3))) void
err(int __errnum, const char *__format, ...);
Copy link
Member Author

Choose a reason for hiding this comment

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

yikes

options/internal/generic/threads.cpp Outdated
// Otherwise we have to set the waiters flag first.
unsigned int desired = expected | mutex_waiters_bit;
if(__atomic_compare_exchange_n((int *)&mutex->__mlibc_state,
reinterpret_cast<int*>(&expected), desired, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED))
if (__atomic_compare_exchange_n(
Copy link
Member Author

Choose a reason for hiding this comment

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

Things like this ought to be factored out

options/posix/generic/pwd.cpp Outdated
p->pw_shell
) < 0
? -1
: 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

cursed ternary

options/rtld/generic/main.cpp Outdated
switch (*aux) {
case AT_PHDR:
mlibc::infoLogger() << "AT_PHDR: 0x" << frg::hex_fmt{*value} << frg::endlog;
break;
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe clang-format off here

@64 64 force-pushed the clang-format branch 3 times, most recently from 34a5b24 to 1bc7598 Compare November 18, 2024 21:07
@64 64 changed the title meta: add clang-format meta: add clang-format and reformat managarm sysdeps Nov 29, 2024
@64 64 force-pushed the clang-format branch 2 times, most recently from 13d5bbc to b6d540c Compare November 30, 2024 00:22
@64 64 added this pull request to the merge queue Nov 30, 2024
Merged via the queue into managarm:master with commit 4b21d4f Nov 30, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dennisbonke Dennisbonke Dennisbonke left review comments

@Geertiebear Geertiebear Geertiebear left review comments

@avdgrinten avdgrinten avdgrinten approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@64 @avdgrinten @Geertiebear @Dennisbonke