UnKn0wnBooof Posted May 4, 2015 Posted May 4, 2015 So, I've been using a program called "FBI" for a while, which lets a user install .CIA files on to the 3DS. There's an option within the application which enables you to recieve a .CIA through WiFi via a sock connection. The dev of the homebrew is using a Java program to send the files. I want a C version so I can port it easily to other devices. I mainly want it so I can run it on Android (compile C as Linux binary, place it in /system/xbin). I looked at some code on other websites but non of which seems to work. Here is the code I'm currently working on: // For both #include <sys/socket.h> #include <stdlib.h> #include <stdio.h> #include <string.h> #include <errno.h> #include <sys/types.h> #include <unistd.h> #include <netinet/in.h> #include <arpa/inet.h> #define PORT 5000 #define BUF_SIZE 16384 int main(int argc, char** argv) { if (argc == 3) { const char* filename = argv[1]; const char* consoleip = argv[2]; FILE *fp = fopen(filename, "ab"); if(NULL == fp) { printf("Error opening file"); return 1; } /* Create a socket first */ int sockfd = 0; if((sockfd = socket(AF_INET, SOCK_STREAM, 0))< 0) { printf("\n Error : Could not create socket \n"); return 1; } /* Initialize sockaddr_in data structure */ struct sockaddr_in serv_addr; serv_addr.sin_family = AF_INET; serv_addr.sin_port = htons(PORT); // port serv_addr.sin_addr.s_addr = inet_addr(consoleip); /* Attempt a connection */ if(connect(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr))<0) { printf("\nError : Connect Failed\n"); return 1; } /* Receive data in chunks of BUF_SIZE bytes */ int bytesReceived = 0; char buff[BUF_SIZE]; memset(buff, '0', sizeof(buff)); while((bytesReceived = read(sockfd, buff, BUF_SIZE)) > 0) { printf("Bytes received %d\n",bytesReceived); fwrite(buff, 1,bytesReceived,fp); } if(bytesReceived < 0) { printf("\nRead Error\n"); } return 0; } else { printf("Usage:\n\n%s [FILENAME] [3DS IP]\n",argv[0]); } return 1; // Something went wrong } I'm new to C so I have no idea what to do. FBI freezes on "Reading info". This is the homebrew: http://gbatemp.net/threads/release-fbi-open-source-cia-installer.386433/ Any help is deeply appreciated. Thanks! Quote
cooper Posted May 4, 2015 Posted May 4, 2015 (edited) I looked at your sources and at both the FBI program and the java Sockfile thing's source code. None of these contain the string "Reading info". Care to try again? When you do, please specify the output you see on the screen on both sides of the communication channel. Regarding your own code, after having received all the data you should fflush() your fp and close both your socket and your fp. Second parameter to fwrite should be sizeof(char). And since bytesReceived being negative is your valid exit situation maybe you shouldn't print "Read error" on encountering it. Edited May 4, 2015 by Cooper Quote
UnKn0wnBooof Posted May 4, 2015 Author Posted May 4, 2015 (edited) I looked at your sources and at both the FBI program and the java Sockfile thing's source code. None of these contain the string "Reading info". Care to try again? When you do, please specify the output you see on the screen on both sides of the communication channel. Regarding your own code, after having received all the data you should fflush() your fp and close both your socket and your fp. Second parameter to fwrite should be sizeof(char). And since bytesReceived being negative is your valid exit situation maybe you shouldn't print "Read error" on encountering it. On the 3DS screen, (in FBI) - it freezes on this: Do you think it would be possible to show a piece of example code? I'm new to C so... Edit: Also, I got "my" code from an example on stack exchange. I have no idea what some of the functions in my script even do. It could be completely wrong for all I know Edited May 4, 2015 by Lavanoid Quote
cooper Posted May 4, 2015 Posted May 4, 2015 (edited) Well for all intents and purposes your code is fine. The only bit of cleanliness that's rubbing me the wrong way is that in C you have low-level IO using int file descriptors and high(er) level io using FILE* pointers. Typically you'd pick one and do everything with it. You're using both (low for the socket, high for the output file). Anyways, let's go over your code since it seems fairly decent. Line 10 - I have no idea what requires that include but it's not one I've seen much. Line 16 - Instead of having a big 'if' whose body is 90% of the program, test for argc not being 3 and if so, spit out an error and return. No need for an 'else' which means you drop 1 level of indentation which is always nice. Lines 18+19 - While it helps to give variables meaningful names, you're not really gaining anything here as you need the variables in question exactly once. Just use the argv[x] value and drop a comment near where you use it for clarity if you must. Line 20 - You're opening a file in (binary) append mode, meaning that if the file already existed you're going to append rather than replace which is almost certainly not what you want. The 'b'(inary) character is allowed for compatibility reasons, but NON-POSIX-compliant. All you need is a simple "w". For low-level IO you'd use int fd = open( argv[1], O_WRONLY|O_EXCL); Line 27 - 'First' relative to what? You created your destination file first... Line 38 - I'd use struct hostent *hostinfo = gethostbyname(argv[2]); instead which also does a DNS lookup if you needed it, the result you can then use to populate sin_addr with, like so: hostinfo = gethostbyname (argv[2]); if (hostinfo == NULL) { printf("Unknown host %s.\n", argv[2]); return 3; // Or whatever. } serv_addr.sin_addr = *(struct in_addr *) hostinfo->h_addr; Line 49 - You use memset to populate the buffer with all '0' characters (0x30 in ASCII). I think you want '\0' instead, which is the NUL character (0x00 in ASCII). Also, don't use sizeof(buff) but instead use sizeof(char)*BUF_SIZE Line 53 - Check your return codes - writes can fail too you know. For low-level IO you use int bytesWritten = write( fd, &buff, bytesReceived); and you should verify that bytesWritten == bytesReceived. Line 57 - As I mentioned before, the situation here is that the remote side closed the socket so you should report it as such. Line 59 - And also as mentioned before, now's the time to close the socket, flush the FILE* pointer if you're using that (file descriptors don't use buffers, so no need to flush them) and (f)close the destination file pointer/descriptor. After that, finally, you can return 0. Note: All code suggestions are untested and I might have a pointer reference mixed up, so don't take it all on face value. Edited May 4, 2015 by Cooper Quote
UnKn0wnBooof Posted May 4, 2015 Author Posted May 4, 2015 Well for all intents and purposes your code is fine. The only bit of cleanliness that's rubbing me the wrong way is that in C you have low-level IO using int file descriptors and high(er) level io using FILE* pointers. Typically you'd pick one and do everything with it. You're using both (low for the socket, high for the output file). Anyways, let's go over your code since it seems fairly decent. Line 10 - I have no idea what requires that include but it's not one I've seen much. Line 16 - Instead of having a big 'if' whose body is 90% of the program, test for argc not being 3 and if so, spit out an error and return. No need for an 'else' which means you drop 1 level of indentation which is always nice. Lines 18+19 - While it helps to give variables meaningful names, you're not really gaining anything here as you need the variables in question exactly once. Just use the argv[x] value and drop a comment near where you use it for clarity if you must. Line 20 - You're opening a file in (binary) append mode, meaning that if the file already existed you're going to append rather than replace which is almost certainly not what you want. The 'b'(inary) character is allowed for compatibility reasons, but NON-POSIX-compliant. All you need is a simple "w". For low-level IO you'd use int fd = open( argv[1], O_WRONLY|O_EXCL); Line 27 - 'First' relative to what? You created your destination file first... Line 38 - I'd use struct hostent *hostinfo = gethostbyname(argv[2]); instead which also does a DNS lookup if you needed it, the result you can then use to populate sin_addr with, like so: hostinfo = gethostbyname (argv[2]); if (hostinfo == NULL) { printf("Unknown host %s.\n", argv[2]); return 3; // Or whatever. } serv_addr.sin_addr = *(struct in_addr *) hostinfo->h_addr; Line 49 - You use memset to populate the buffer with all '0' characters (0x30 in ASCII). I think you want '\0' instead, which is the NUL character (0x00 in ASCII). Also, don't use sizeof(buff) but instead use sizeof(char)*BUF_SIZE Line 53 - Check your return codes - writes can fail too you know. For low-level IO you use int bytesWritten = write( fd, &buff, bytesReceived); and you should verify that bytesWritten == bytesReceived. Line 57 - As I mentioned before, the situation here is that the remote side closed the socket so you should report it as such. Line 59 - And also as mentioned before, now's the time to close the socket, flush the FILE* pointer if you're using that (file descriptors don't use buffers, so no need to flush them) and (f)close the destination file pointer/descriptor. After that, finally, you can return 0. Note: All code suggestions are untested and I might have a pointer reference mixed up, so don't take it all on face value. Wow! Thanks for the tips! As I said - most (if not all) of the code I provided was copied 'n pasted from stackexchange. I made minor modifications. I was pretty much guessing stuff . I really appreciate the help! I contacted the dev of FBI and he also provided some code and we spent about half an hour fixing stuff so it would compile under Linux. Here is the new code: #include <errno.h> #include <string.h> #ifdef __WIN32__ #include <winsock2.h> char* sockGetErrorString() { char *s = NULL; FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, NULL, (DWORD) WSAGetLastError(), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPSTR) &s, 0, NULL); return s; } #else #include <arpa/inet.h> char* sockGetErrorString() { return strerror(errno); } #endif #include <stdio.h> #include <fcntl.h> #include <unistd.h> #include <string> #include <stdlib.h> int main(int argc, const char* argv[]) { if(argc != 3) { printf("Usage: %s ip file\n", argv[0]); return -1; } #ifdef __WIN32__ WORD versionWanted = MAKEWORD(1, 1); WSADATA wsaData; WSAStartup(versionWanted, &wsaData); #endif FILE* fd = fopen(argv[2], "r"); if(!fd) { printf("Failed to open file: %s\n", sockGetErrorString()); #ifdef __WIN32__ WSACleanup(); #endif return -1; } fseek(fd, 0, SEEK_END); uint64_t size = (uint64_t) ftell(fd); fseek(fd, 0, SEEK_SET); int sock = socket(AF_INET, SOCK_STREAM, 0); if(sock < 0) { printf("Failed to open socket: %s\n", sockGetErrorString()); #ifdef __WIN32__ WSACleanup(); #endif return -1; } struct sockaddr_in address; memset(&address, 0, sizeof(address)); address.sin_family = AF_INET; address.sin_port = htons(5000); address.sin_addr.s_addr = inet_addr(argv[1]); if(connect(sock, (struct sockaddr *) &address, sizeof(address)) < 0) { printf("Failed to connect: %s\n", sockGetErrorString()); #ifdef __WIN32__ WSACleanup(); #endif return -1; } printf("Sending info...\n"); fflush(stdout); uint64_t fileSize = size; static const int num = 42; if(*((char*) &num) == num) { fileSize = (((uint64_t) htonl((uint32_t) fileSize)) << 32) + htonl((uint32_t) (fileSize >> 32)); } if(send(sock, (char*) &fileSize, sizeof(fileSize), 0) < 0) { printf("Failed to send info: %s\n", sockGetErrorString()); #ifdef __WIN32__ WSACleanup(); #endif return -1; } printf("Sending file...\n"); fflush(stdout); uint64_t bufSize = 1024 * 16; // 16KB void* buf = malloc(bufSize); for(uint64_t pos = 0; pos < size; pos += bufSize) { size_t read = fread(buf, 1, bufSize, fd); if(send(sock, (char*) buf, read, 0) < 0) { printf("Failed to send file (pos %d): %s\n", pos, sockGetErrorString()); #ifdef __WIN32__ WSACleanup(); #endif return -1; } } printf("Waiting for server to finish receiving...\n"); char temp; while(recv(sock, &temp, sizeof(temp), 0) != 0) { sleep(1); } printf("Closing...\n"); close(sock); fclose(fd); printf("File successfully sent.\n"); #ifdef __WIN32__ WSACleanup(); #endif return 0; } You think any improvements could be made to this? Quote
cooper Posted May 5, 2015 Posted May 5, 2015 I'm a bit puzzled. This code you now posted is for sending a file whereas the other was for receiving a file. Did you now post the other side of the communication channel, or did you morph your program into this precisely because it was to send a file? So looking at this the 'protocol' is to first send 64 bits to indicate the size of the file that is getting sent, followed by the file data itself. Lines 46-48 - If you want to do this with low-level IO you use the lseek function or, prior to even opening the file, using stat() to also verify the file even exists and is, in fact, a file and not a folder or whatever. Lines 77-79 - That condition looks mighty funky, but I get it. The int is 32 bit and if you make its address a char pointer, then access it as a char you get the topmost 8 bits in accordance with how your platform orders its memory. If you get the same value in the char as in the int, the least significant bytes come first in memory, otherwise the numerical value of the char is 0 and you know the most significant bytes come first in memory. Clever. Lines 73+90 - That fflush shouldn't be needed. Lines 105-110 - Why? This is TCP. Data which you put in transit *WILL* arrive even after you closed the socket and go and do other things. With this code in place you're effectively waiting for the other end to close the socket which provides no gain at all for you. Just close and be done with it. Quote
UnKn0wnBooof Posted May 5, 2015 Author Posted May 5, 2015 Ah, my oroginal code was meant to be doing what the second piece of code does - send a file to the 3DS. The code I posted wasn't meant to be recieving anything. Since I had copied the first bunch of code from stackexchange, I had thought that it would be doing what the second piece of code does. Both pieces of code are meant to be sending a file, yes :) . Like I said - new to C, I pretty much know nothing. I'm definitely gonna note down your responses though, as they may come in handy when I try to learn more about it. The second piece of code I posted, was made by the developer of FBI. FBI (3DS Homebrew) receives the file while the code I posted was designed to send it. Really appreciate your response :D Do you do console hacking by any chance? You seem quite knowledgeable about this stuff! Why not make a homebrew? With your skills, you'd be an awesome addition to the GBATemp community! Quote
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.