Re: [ros-dev] [ros-diffs] [ion] 23175: - Major formatting cleanup and optimizations to NtQueryInformationProcess so that it does not burn the eyes of anyone that might attempt to read it.

[Royce Mitchell III]

ion(a)svn.reactos.org wrote:

>  DstBuffer = (PWSTR)(DstPath + 1);
>   
>  
that looks pretty eye-burning itself... perhaps I'm just seeing things but,

1) you're incrementing a pointer without checking if it's null ( I 
realize this might be safe if we can trust our caller ).

2) do we really want the address: DstPath + sizeof(PUNICODE_STRING)? I 
realize this too might work, but do we want to assume that 
sizeof(PUNICODE_STRING) == 2*sizeof(USHORT) ???

[Alex Ionescu]

Royce Mitchell III wrote:
>   ion(a)svn.reactos.org wrote:
>  
>  
> >  DstBuffer = (PWSTR)(DstPath + 1);
> >  
> >  
>  
>  that looks pretty eye-burning itself... perhaps I'm just seeing things but,
>  
>  1) you're incrementing a pointer without checking if it's null ( I 
>  realize this might be safe if we can trust our caller ).
>  
>  2) do we really want the address: DstPath + sizeof(PUNICODE_STRING)? I 
>  realize this too might work, but do we want to assume that 
>  sizeof(PUNICODE_STRING) == 2*sizeof(USHORT) ???
>   
You should notice I didn't actually write this code, so "I"'m not doing
any of this. Yes, the function makes dangerous assumption and is poorly
coded, but if you'll notice the FIXME above it, it's already completely
wrongly implemented. Nevertheless, thanks for the comments and looking
out :)

1) We trust him, we've validated the pointer earlier.
2) sizeof(PUNICODE_STRING) == 2*sizeof(USHORT) is not correct btw. And
yes, I think that this function is made so that the buffer follows right
after the unicode_string structure.

If you test on XP, you'll notice this similar behavior.

Best regards,
Alex Ionescu