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

lwNBDsvr: Small refactoring #506

Merged
merged 1 commit into from
Aug 24, 2021
Merged

lwNBDsvr: Small refactoring #506

merged 1 commit into from
Aug 24, 2021

Conversation

bignaux
Copy link
Member

@bignaux bignaux commented Aug 24, 2021

Pull Request checklist

Note: these are not necessarily requirements

  • I reformatted the code with clang-format
  • I checked to make sure my submission worked
  • I am the author of submission or have permission from the original author
  • Requires update of the PS2SDK
  • Requires update of the gsKit
  • Others (please specify below)

Pull Request description

@rickgaiser
Copy link
Member

Please take a look at the codacy issues. Let me know when you're satisfied, then I'll pull the changes.

@bignaux bignaux force-pushed the nbd branch 4 times, most recently from a88e4b7 to 76db1d7 Compare August 24, 2021 16:39
@bignaux
Copy link
Member Author

bignaux commented Aug 24, 2021

Ok for me, still better. I get some ATAD error sometime like :

ATA device driver v1.2 - Copyright (c) 2003 Marcus R. Brown
atad: Driver loaded.
atad: Error: PIO cmd error: status 0x51, error 0x04.
atad: error: ATA failed, -503

on ata_get_devinfo but seems harmless and

atad: Error: DMA timeout.
atad: error: ATA failed, -502
atad: Timeout while waiting on busy (0x88).
atad: Timeout while waiting on busy (0x88).
atad: Timeout while waiting on busy (0x88).
atad: Timeout while waiting on busy (0x88).
atad: Timeout while waiting on busy (0x88).
``` during some transfert that could be better handle

@rickgaiser rickgaiser merged commit 6d42fad into ps2homebrew:master Aug 24, 2021
size = nbd_send(client_socket, &new_hs, sizeof(struct nbd_new_handshake),
0);
if (size < sizeof(struct nbd_new_handshake))
goto error;
Copy link
Member

Choose a reason for hiding this comment

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

is it possible to get rid of all goto? Something like return ctx, or return null. Goto can lead to some problems in the future.

Copy link
Member

Choose a reason for hiding this comment

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

If you want more description, you can define retcodes somewhere like
RET_ERROR = 0;
RET_SOFTDISCONNECT = -1;

Copy link
Member

Choose a reason for hiding this comment

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

break;
// see nbdkit send_newstyle_option_reply_exportnames()
case NBD_OPT_LIST:

Copy link
Member

Choose a reason for hiding this comment

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

for strlen warnings, maybe just for safety:

ctx->export_name[32] = '\0';
ctx-> export_desc[64] = '\0';

we use static arrays, I dont think that we will ofill the whole string, but for safety this can be assigned

Copy link
Member

Choose a reason for hiding this comment

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

@@ -397,6 +103,8 @@ int nbd_init(struct nbd_context **ctx)

while (1) {

printf("lwNBD: a new client connected.\n");
Copy link
Member

Choose a reason for hiding this comment

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

Maybe all printf do only ifdef DEBUG ?? printf is a bit heavy function, can cause timeouts and slowdowns

Copy link
Member

Choose a reason for hiding this comment

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

or use dbgprintf instead

AKuHAK pushed a commit that referenced this pull request Sep 30, 2021
lwNBDsvr: Small refactoring
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
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