[Top][All Lists]

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

Re: store_set_size (patch)

From: Marcus Brinkmann
Subject: Re: store_set_size (patch)
Date: Sun, 29 Sep 2002 02:24:12 +0200
User-agent: Mutt/1.4i

On Mon, Sep 23, 2002 at 07:10:06PM +0200, Ludovic Court├Ęs wrote:
> Hi,
> Here is another updated patch for libstore (which adds a store_set_size ()
> call).

This is basically ok.  We will need copyright assignments from you before we
can put it in, though.  Can you work out the details with 
You have the options of just assigning the copyright for this change (or
even disclaim only) or also for future changes.

As for the patch itself:  Please only put one changelog entry if it belongs
together, as we didn't put it in yet, we are not interested in the history
of the patch itself.

I don't fancy the file_set_size_ function name.  The trailing _ is easy to
miss.  Maybe file_our_set_size or something like that (eg, keep the usual
conventions for symbol names).

Avoid comments like /* ... */.  The ellipse here doesn't tell me what it
stands for.  Use things like /* XXX Implement this.  */  or similar.

If you have a block and a comment like here:

> +    /* See if the the current (one) range is that the kernel is enforcing. */
> +    {

... put the comment inside the block.

> +  if (! err)
> +  {

and always honor the GCS :)

> +static error_t
> +zero_set_size (struct store *store, store_offset_t newsize)
> +{
> +  return 0;
> +}
> +

Are you sure this is correct?  If you set the size, you might expect it to
fail if you try to read outside the size you set.

Can you please make the above changes and start the copyright assignment
process?  Then we can put it in.


`Rhubarb is no Egyptian god.' GNU
Marcus Brinkmann              The Hurd

reply via email to

[Prev in Thread] Current Thread [Next in Thread]