25
Style: Indent your operands to a consistent column, so mnemonics of different length don't make your code look so ragged. And use local .label labels inside functions.
Comment code that depends on non-standard behaviour: stdout is only guaranteed to be line-buffered, and isn't automatically flushed when you read stdin in ISO C. Some systems (like Linux) ...
18
There are numerous small things.
You don't have to keep subtracting and adding 32 from rsp. Allocate the space once at the start of the function (main), reuse it for the duration, and add it back at the end (but see below).
My personal preference would be to use mov ecx,offset question to make it clear that I want the address of the variable, and not the ...
16
You can simplify paths using forward slashes:
GOOGLE_DRIVE_DIRECTORY = 'C:/Users/Jeff/Google Drive/Manifest_Destiny'
MAIN_BACKUP_DIRECTORY = 'C:/Users/Jeff/Desktop/Manifest_Destiny_Backups/md_backup_{0}'
EXTERNAL_DRIVE_DIRECTORY = 'F:/My Files/Manifest_Destiny_Backups/md_backup_{0}'
Instead of converting a date to a string, which may be locale dependent ...
15
Collision detection
You can calculate the collision much easier with the Rectangle structure.
Use the Contains method if you are interested in single points that
Determines if the specified point is contained within this Rectangle structure.
var pictureRect = new Rectangle(
pictureBox.Location.X,
pictureBox.Location.Y,
pictureBox.Width,
...
14
The comments in the clearmem procedure for the Linux block look a bit confusing. You could just have a summary of the procedure commented above, and have the individual comments for each line specify the meaning of the assembly instructions.
Specifically, the lines that describe the C++ code don't quite reflect on the assembly code. clearmem itself doesn'...
13
I can't say I particularly like this code as it is right now. It seems to me that there are two reasonable approaches: if you think most of what you zero will be in main memory, then you probably just want the most compact code possible for the job. If you think it'll be used to zero data that might be in the cache a noticeable amount of the time, then you ...
13
You register keydown events for Control and Alt keys, but don't note when they are released. Therefore, you would not be able to distinguish between r and Controlr by reading the transcript.
Those are some huge switch statements. I would consider making some lookup tables instead. There are fewer than 255 key codes defined, and their numeric values are ...
13
Using constants
Rather than using #define and creating a bunch of macros like this:
#define PLANEHEIGHT 20
#define PLANEWIDTH 50
#define INITIALSNAKELENGTH 3
#define FPS 5
You should use const instead, like this:
const int PLANE_HEIGHT = 20;
const int PLANE_WIDTH = 50;
const int INITIAL_SNAKE_LENGTH = 3;
const int FRAMES_PER_SECOND = ...
12
You're actually writing in C but using some C++ features, I'd drop C style (and functions) in favor of their C++ counterparts:
1) Even if you're using a C API you should write C++ classes to hide them. Ratchet already addressed this issue in his answer then I won't repeat again here.
2) You're using sscanf to parse a string, you should use std::...
11
I'm not excited about the structure of the code. I think you're putting more in one class than is really a good idea.
Just for example, I'd start with a a tiny class that does nothing but handle the WSAStartup/WSACleanup sequence:
class WSAUser {
WSADATA data;
public:
WSAUser() { /* call WSAStartup() */ }
~WSAUser() { /* call WSACleanup() */ }
...
11
Pretty sure this is not portable.
long length = htonl( i.length() );
ret = ::send( hSocket, reinterpret_cast<const char*>(&length), sizeof(length), 0 );
You are obviously trying to make it portable (good). But htonl() does not return a long which is platform/compiler dependent. What you want really want is uint32_t (man htonl).
Anything ...
11
I think you have too many different responsibilities bound up into one class. That ends up not only making that class more complex, but adding extra complexity overall as well. As a starting point, I'd have a really trivial class that does nothing but stack-based management of the WSAStartup/WSACleanup:
struct socket_user {
socket_user() {
...
11
You have some global variables in main.c:
bool invisible = true;
char fileName[MAX_PATH];
They should be placed in the closest local scope possible and passed to functions as needed.
Your #defines in keylogger.c:
#define VK_VOLUME_MUTE 0xAD
#define VK_VOLUME_DOWN 0xAE
#define VK_VOLUME_UP 0xAF
can be made into a more concise enum:
typedef enum { MUTE=...
11
You have committed a grave error here with operator precedence, which should have easily been caught with simple testing:
X1 = Sector4Arithmetic / 2 * a;
The variable names SectornArithmetic are bizarre. Most of those intermediate results don't need to be assigned to a variable. The most complex intermediate result, b * b - 4 * a * c, has a standard name:...
10
One variable per line please.
int gess,i=0,found=0, r = rand()%100;
This makes the code hard to read. There is also one corner case (with pointers) were this will not work as expected for beginners. As a result this is usually banned in most companies coding standards.
You are using found as a boolean variable.
Treat it as such. use TRUE/FALSE.
I would ...
10
Copy&pasting code multiple times does not make it more secure. If we clean up your mutate_seed method, we'd end up with this:
def mutate_seed(self):
for x in range(self.seed):
number_or_letter = r.randint(0, 1)
if number_or_letter:
self.mutated_seed.append(r.randint(0, 9))
else:
lower_or_higher = r....
10
Your code can benefit from RAII. For example by creating a small HInternet struct with a destructor to hold a single HINTERNET. Then you don't need to worry about calling InternetCloseHandle in the correct places as the destructor will do it for you.
struct HInternet{
HInternet(HINTERNET handle) : handle(handle){}
HInternet() : handle(nullptr){}
...
10
For the if(filePath.Length >= 240) part, you can use the Path object.
It throws a PathTooLongException.
You can then change to this :
try
{
filePath = Path.GetFullPath(filePath);
}
catch (PathTooLongException ex)
{
directoryFound = false;
Console.WriteLine("Please keep the filepath under 240 chars so that you still are able to provide a name ...
10
Here are some suggestions for how you might improve your code.
Use the appropriate #includes
This program fragment requires headers, which should be included:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Use const where practical
Because the validate_path function does not alter the passed string, the parameter should be ...
10
General
Try to comply with PEP-8. PEP-8 is the name of the official Python style guide. Complying with it makes your code easier to read. Ultimately, developers can more easily collaborate on a project.
Every import-statement should be on a separate line. The only exception is importing 'from' a module / package. Furthermore, imports should be grouped:
...
9
It's very fast, but I wonder if hooking into the API (DirectX) and getting the buffer will be faster than this (I don't know how to do that, though).
I don't know either.
What you have here, probably hasn't been reviewed yet, because there's essentially nothing blatantly wrong with it.
What you've done well
Pretty much everything, as far as I can tell:
...
9
My main comment is that you need to concentrate on avoiding duplication of
code and on writing smaller functions. 50 lines or so is the sort of max length I use. You also use long names that are often
too long giving the code a very dense appearance and making it difficult ot
read. Smaller functions, reduced variable scope and hence shorter name sizes
...
9
WSAGetLastError() returns an int. Throwing an int is legal but not quite usual. Your catch(std::exception& e) statements are fairly useless, given that Win32 APIs are unlikely to throw a std::exception object.
Your default constructor is dangerous: you can't tell if a socket instance has been properly-constructed or default-constructed. You should ...
9
Your specification has holes in it.
What will be if user passes NULL? Undefined behavior? Return?
What if the path is already valid? Copy the string? Return itself? The latter case is probably unexpected, and will make following code to cause UB:
char* path = malloc(...);
char* valid_path = validate_path(path);
free(path); //tsunami is probably created ...
8
There are at least a few things I think I'd do differently, at any rate.
const int MAX_RECV_LEN = 8096;
const int MAX_MSG_LEN= 1024;
const int PORT_NUM = 1200;
The ALL_CAPS convention was originally invented for macros. At least as far as I can tell, it was originally for function-like macros, to give a subtle warning that it might evaluate its argument ...
8
The class name is misspelled - should be Socket rather than Sokect
Consider putting your constant definitions as static members into the class like this:
class Socket
{
private:
static const int MAX_RECV_LEN = 8096;
static const int MAX_MSG_LEN = 1024;
static const int PORT_NUM = 1200;
....
}
You haven't shown the actual ...
8
Your public single-argument constructor Sokect(int); absolutely must be marked explicit unless you wish to allow passing int values where Sokect objects are expected. See some of these answers for more detail on how this works.
8
Expected behaviour / User experience
Logging passwords to a file in unhashed, unencrypted form is bad practice. Doing so without telling the user is a sin.
Using cls to clear the screen is non-portable.
There is no clean way to exit the infinite loop in your program. I think that the following conditions should all trigger a clean exit:
ControlC (...
8
Here's a few things I noted:
Don't have using namespace std; in your code. It's almost universally considered a bad practice.
The C++ standard states that you should use int main() and not void main(). Bjarne Stroustrup (a creator of C++) stated that this isn't even C++ (or even C!). I'm surprised you could even get this to work, my compiler won't even ...
8
Loop like a native
I suggest you have a look/read at Ned Batchelder's excellent presentation "Loop like a native".
In your case, any time you write:
for i in range(0, (len(search_installed.Updates))):
# saves installed update name in a variable
update_installed = search_installed.Updates.Item(i)
You should most probably write:
for ...
Only top voted, non community-wiki answers of a minimum length are eligible
