Re: [PATCH] Strbuf documentation: document most functions

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

 



Hi,

I am no author of strbuf, but hey, I thought I'd just give you a few 
comments...

In general, I'd rather leave the "->" from the members, since you have 
many instances where you access them with ".".

On Tue, 3 Jun 2008, Miklos Vajna wrote:

> diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
> index a52e4f3..3879e0e 100644
> --- a/Documentation/technical/api-strbuf.txt
> +++ b/Documentation/technical/api-strbuf.txt
> @@ -1,6 +1,175 @@
>  strbuf API
>  ==========
>  
> -Talk about <strbuf.h>
> +strbuf's can be use in many ways: as a byte array, or to store arbitrary
> +long, overflow safe strings.

I think that you should not suggest using strbufs as byte array, even if 
that is certainly possible.  Rather, you should say something like:

	An strbuf is NUL terminated for convenience, but no function in 
	the strbuf API actually relies on the string being free of NULs.

> +strbufs has some invariants that are very important to keep in mind:
> +
> +. The `->buf` member is always malloc-ed, hence strbuf's can be used to
> +  build complex strings/buffers whose final size isn't easily known.

Is this true?  I thought the initial string is empty, but not alloc'ed.

So I'd rather have something like

	The "buf" member is never NULL, so you can safely strcmp() it.

I'd like to see a comment that strbuf's _have_ to be initialized either by 
strbuf_init() or by "= STRBUF_INIT" before the invariants, though.

> +`strbuf_attach`::
> +
> +	Attaches a string to a buffer. You should specify the string to attach,
> +	the current length of the string and the amount of allocated memory.

... This string _must_ be malloc()ed, and after attaching, the pointer 
cannot be relied upon anymore, and neither be free()d directly.

> +`strbuf_read`::
> +
> +	Read the contents of a given file descriptor. The third argument can be
> +	used to give a hint about the file, to avoid reallocs.

s/about the file/& size/

> +`strbuf_read_file`::
> +
> +	Read the contents of a file, specified by its path. The third argument
> +	can be used to give a hint about the file, to avoid reallocs.

Ditto.

> +`strbuf_getline`::
> +
> +	Read a line from a FILE* pointer. The second argument specifies the line
> +	terminator character, like `'\n'`.

s/like/typically/

Thanks,
Dscho

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux