-
Notifications
You must be signed in to change notification settings - Fork 439
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
Incremented buffer size for ignored files #99
Incremented buffer size for ignored files #99
Conversation
Please, hold on this pull request. When I have time, I'll update it once more to have dynamic-allocated buffer. |
@ladislav-zezula - yes, you are right, dynamic buffer will be much better. I used the static one temporarily, because I thought the cases where it will exceed that limit are going to be rare enough. but it seems it reached to this point when I should implement it in a proper way. I am going to take care of this soon. but if you come up with the working solution before, of course you are welcome. |
I can also add an option of supplying the ignore list as a file. If there are so many entries, I think it will come more handy. |
@ladislav-zezula - check this: https://github.com/hasherezade/pe-sieve/tree/mignore_refact added a new parameter |
I'll do it and then you can add your intended
That would indeed be cool. Maximum length of the command like in Windows is 0x7FFF characters, so it will eventually run out :-)
I don't see any recent commit there. I can use that branch as target for my merge request tho. |
* Fixed some MSVC warnings
Ok I turned both fixed-size buffers ( I also fixed all MSVC warnings, except one, see below. MS Visual studio really doesn't like that one and spits like a page full of description about how is this bad. It may be worth a separate pull request, tho:
You may add your new parameter on top of this. |
@ladislav-zezula - the point is, neither Those limitations are not present in HollowsHunter, that's why it do have parameters defined as strings. |
Oh, I see. I didn't know that. Please, tell me - are buffers allocated by new[] OK? I mean, will there always be destructor who will free the buffers? |
@ladislav-zezula - no, we cannot rely on |
Well it doesn't need to be new/delete, but malloc/free (or even HeapAlloc/HeapFree) would be perfectly fine. But if you want an UNICODE_STRING, that would be fine too, except that UNICODE_STRING has limit of 32767 characters. |
I get it, you just want that allocation and deallocation of the string will be fully on the user. We can do this, but then it will require additional verification of the buffer. typedef struct _PARAM_STRING {
size_t Length;
char* Buffer;
} PARAM_STRING; I will experiment with it a bit, and propose some code changes today. |
@ladislav-zezula - check this out: https://github.com/hasherezade/pe-sieve/tree/mignore_refact2 |
Looks good. One more note - you may also wanna consider making the output directory UNICODE and also dynamic - in that case, potential length of path is up to 32767. However, currently you use ANSI strings, so unless you want to support Asian languages, it looks good to me. |
@ladislav-zezula - ok, I merged my changes to the master. they are available in both pe-sieve and hollowshunter. |
The feature looks good. Good job, thank you! |
Hi, we've come to the point where the amount of files that we need to ignore has reached its limit. As a temporary fix, I incremented the size of the buffer to 2048, doubled from previous 1024.
It may be worth it to replace it with some dynamic sized variable, however.