Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 26 Jul 2013 04:38:01 +0200
From: Lukas Odzioba <lukas.odzioba@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: cryptocurrency mining fallback in JtR

2013/7/18 Rafael Waldo Delgado Doblas <lord.rafa@...il.com>:
> Hello, this is the link to my JtR fork,
> https://github.com/LordRafa/JohnTheRipper

Rafael I took a look at your commits and here are my comments.

Commit 04056a2c1f aka "added modules cgminer and dtach" adds 138k
lines of code. I am affraid that this kind of "integration" in
unacceptable and for sure we will not merge this change to
bleeding-jumbo. I guess you need to have those sources inside john's
src but it would be much better to add a script that downloads them
from github.

miner_wrap.c:
#include "stdio.h"
It should be <stdio.h> like two other includes, there is no need to
search for this header in current directory.

Code should check whether there are as many args as expected, print
usage example and quit if you specify less than expected.

fork() and execve() may return -1 on exit, and you should handle that,
Solar mentioned that earlier.

I think that there might me some problems with cleaning out envp, I
guess it should be a copy of current process envp.

miner.c:
int get_data(...) function can be void get_data(...) since it can only return 1

gethostbyname() returns NULL on error and your code will likely crash then.
strchr() - same as above

Please improve send_data/CLOSESOCKET() logic in for (i = 0; i <
numgpu; i++) loop (starts at line 104).
It would be good to SEE when what was closed.

It would be better to have some of those magic strings like "AfterEnd"
#defined in miner.h.
I would be happy to see miner_start() function splited into two
separate static functions now it mixes logic of two separate features.

Also please read once again Solar's comments to your code in this
thread, I think you missed some of them.

Lukas

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.