Hi Jeff,
Since the whole nic structure is zeroed out initially, the
"block_virt_addr" is initially NULL. So, if the allocation
succeeds, it will hold a non-NULL value.
So, in the freeSharedMem() if we check for it's non-NULL
value and free it, we should be okay. Do you agree ?
Thanks,
Ravi
-----Original Message-----
From: Jeff Garzik [mailto:jgarzik@xxxxxxxxx]
Sent: Thursday, February 19, 2004 6:41 PM
To: ravinandan arakali
Cc: raghavendra.koushik@xxxxxxxxx; leonid.grossman@xxxxxxxx;
netdev@xxxxxxxxxxx; raghavendra.koushik@xxxxxxxx
Subject: Re: Submission for S2io 10GbE driver
ravinandan arakali wrote:
> Hi Jeff,
> For #7, the temporary variable tmp_v_addr is assigned as follows
> (immediately below the code you have quoted):
>
> nic->rx_blocks[i][j].block_virt_addr = tmp_v_addr;
Yes... but only after the allocation.
> In the freeSharedMem() routine, we go thru' each block of each
> receive ring and free it. Following is the relevant piece of
> code:
> for (i = 0; i < config->RxRingNum; i++) {
> blk_cnt = nic->block_count[i];
> for (j = 0; j < blk_cnt; j++) {
> tmp_v_addr = nic->rx_blocks[i][j].block_virt_addr;
> tmp_p_addr = nic->rx_blocks[i][j].block_dma_addr;
> pci_free_consistent(nic->pdev, size, tmp_v_addr,
tmp_p_addr);
> }
> }
>
> But I guess in the above piece of code, it may be a good idea to
> check if nic->rx_blocks[i][j].block_virt_addr is non-NULL before
> doing the pci_free_consistent().
Yes, you could add the check there.
Jeff
|