Re: [ros-dev] [ros-diffs] [zhu] 71823: Fixed crash on client and server exit: Corrected some NULL data marking and checking. Moved some cleanup code so they actually execute on function failure (incomplete). Removed redunda...

[Thomas Faber]

Comments inline.

On 2016-07-05 20:35, zhu(a)svn.reactos.org wrote:
>   ---
> branches/GSoC_2016/lwIP-tcpip/drivers/network/tcpip/address.c	[iso-8859-1] (original)
>  +++ branches/GSoC_2016/lwIP-tcpip/drivers/network/tcpip/address.c	[iso-8859-1] Tue Jul  5
> 18:35:17 2016
>  @@ -357,15 +356,15 @@
>   			
>   			KeReleaseSpinLockFromDpcLevel(&Context->RequestListLock);
>   			
>  -			if (Context->lwip_tcp_pcb != Context->AddressFile->lwip_tcp_pcb)
>  +			if (Context->lwip_tcp_pcb == Context->AddressFile->lwip_tcp_pcb); 
You really don't want a semicolon here ;)

>    			{
>  -				tcp_close(Context->lwip_tcp_pcb);
>  +				Context->AddressFile->lwip_tcp_pcb = NULL;
>   			} 

>   +	if (!p)
>  +	{
>  +		CopiedLength = 0;
>  +		Status = STATUS_ADDRESS_CLOSED;
>  +		goto BAD;
>  +	}
>   	
>   	if (RemainingDestBytes <= p->len)
>   	{
>   		RtlCopyMemory(CurrentDestLocation, p->payload, RemainingDestBytes);
>   		CopiedLength = RemainingDestBytes;
>  +		Status = STATUS_SUCCESS;
>   		goto RETURN; 
We normally call labels things like "Exit" and "Failure".

>   @@ -826,23 +846,24 @@
>   	
>   	tcp_recved(tpcb, CopiedLength);
>   	 
I find it confusing not to have Entry = Head->Flink right here. I'm not
sure where it is, but unless there's a good reason for it to be
somewhere else I'd recommend putting it right in front of the loop.

>   +	while (Entry != Head)
>  +	{
>  +		Request = CONTAINING_RECORD(Entry, TCP_REQUEST, ListEntry); 
Entry = Entry->Flink? ;)

>   +		if (Request->PendingMode ==
> TCP_REQUEST_PENDING_RECEIVE)
>  +		{
>  +			tcp_recv(tpcb, lwip_tcp_receive_callback);
>  +			break;
>  +		}
>  +	} 




>   @@ -1666,6 +1690,21 @@
>   	if (!(IsListEmpty(&Context->RequestListHead)))
>   	{
>   		DPRINT1("Disassociating context with outstanding requests\n");
>  +		Head = &Context->RequestListHead;
>  +		Entry = Head->Flink;
>  +		while (Entry != Head)
>  +		{
>  +			Request = CONTAINING_RECORD(Entry, TCP_REQUEST, ListEntry); 
Entry = Entry->Flink;

>   +			if (Request->PendingIrp)
>  +			{
>  +				IrpSp = IoGetCurrentIrpStackLocation(Request->PendingIrp);
>  +				DPRINT1("Pending IRP Control Code: %08x\n", IrpSp->MinorFunction);
>  +			}
>  +			else
>  +			{
>  +				DPRINT1("IRP is NULL\n");
>  +			}
>  +		}
>   	}
>   	KeReleaseSpinLockFromDpcLevel(&Context->RequestListLock);
>   	 

Thanks.
-Thomas