[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 at 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