Re: [PATCH 1/5] CIFS: Move buffer allocation to a separate function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 24, 2011 at 10:51:53AM +0400, Pavel Shilovsky wrote:
> +static bool
> +allocate_buffers(char **pbigbuf, char **psmallbuf, unsigned int size,
> +		 bool isLargeBuf)

No p-prefixes for pointers, and no camelCase, please.

> +{
> +	char *bigbuf = *pbigbuf, *smallbuf = *psmallbuf;
> +
> +	if (bigbuf == NULL) {
> +		bigbuf = (char *)cifs_buf_get();
> +		if (!bigbuf) {
> +			cERROR(1, "No memory for large SMB response");
> +			msleep(3000);
> +			/* retry will check if exiting */
> +			return false;
> +		}
> +	} else if (isLargeBuf) {
> +		/* we are reusing a dirty large buf, clear its start */
> +		memset(bigbuf, 0, size);
> +	}
> +
> +	if (smallbuf == NULL) {
> +		smallbuf = (char *)cifs_small_buf_get();
> +		if (!smallbuf) {
> +			cERROR(1, "No memory for SMB response");
> +			msleep(1000);
> +			/* retry will check if exiting */
> +			return false;
> +		}
> +		/* beginning of smb buffer is cleared in our buf_get */
> +	} else /* if existing small buf clear beginning */
> +		memset(smallbuf, 0, size);
> +
> +	*pbigbuf = bigbuf;
> +	*psmallbuf = smallbuf;
> +
> +	return true;

But in the end this whole code doesn't make any sense at all.  Just
sleeping when failing to memory is a bad idea, as is all this partial
reuse stuff.

Someone needs to rearchitect this code to actually make sense.

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux