[PATCH] Really *do* nothing in while loop
Signed-Off-by: Thomas Glanzmann <[email protected]>
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -335,7 +335,7 @@
stream.next_in = hdr;
stream.avail_in = hdrlen;
while (deflate(&stream, 0) == Z_OK)
- /* nothing */
+ /* nothing */;
/* Then the data itself.. */
stream.next_in = buf;
Thomas Glanzmann wrote:
> [PATCH] Really *do* nothing in while loop
>
> Signed-Off-by: Thomas Glanzmann <[email protected]>
>
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -335,7 +335,7 @@
> stream.next_in = hdr;
> stream.avail_in = hdrlen;
> while (deflate(&stream, 0) == Z_OK)
> - /* nothing */
> + /* nothing */;
>
> /* Then the data itself.. */
> stream.next_in = buf;
Well, the lack of semicolon is wrong really (and funny).
But is the whole while loop needed at all? deflate()
consumes as much input as it can, producing as much output
as it can. So without the loop, and without updating the
buffer pointers ({next,avail}_{in,out}) it will do just
fine without the loop, and will return something != Z_OK
on next iteration. If this is to mean to flush output,
it should be deflate(&stream, Z_FLUSH) or something.
/mjt
P.S. What's [email protected] for ?
Hello,
> >- /* nothing */
> >+ /* nothing */;
> Well, the lack of semicolon is wrong really (and funny).
yes, it is but harmless in this envrionment.
> But is the whole while loop needed at all? deflate()
> consumes as much input as it can, producing as much output
> as it can. So without the loop, and without updating the
> buffer pointers ({next,avail}_{in,out}) it will do just
> fine without the loop, and will return something != Z_OK
> on next iteration. If this is to mean to flush output,
> it should be deflate(&stream, Z_FLUSH) or something.
I have no idea.
> P.S. What's [email protected] for ?
It is the list which handles GIT related discussions. Frontend/backend
and isn't kernel related.
Thomas
On Sun, 2005-05-08 at 19:48, Michael Tokarev wrote:
> Thomas Glanzmann wrote:
> > [PATCH] Really *do* nothing in while loop
> >
> > Signed-Off-by: Thomas Glanzmann <[email protected]>
> >
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -335,7 +335,7 @@
> > stream.next_in = hdr;
> > stream.avail_in = hdrlen;
> > while (deflate(&stream, 0) == Z_OK)
> > - /* nothing */
> > + /* nothing */;
> >
> > /* Then the data itself.. */
> > stream.next_in = buf;
>
> Well, the lack of semicolon is wrong really (and funny).
>
> But is the whole while loop needed at all? deflate()
> consumes as much input as it can, producing as much output
> as it can. So without the loop, and without updating the
> buffer pointers ({next,avail}_{in,out}) it will do just
> fine without the loop, and will return something != Z_OK
> on next iteration. If this is to mean to flush output,
> it should be deflate(&stream, Z_FLUSH) or something.
>
> /mjt
>
> P.S. What's [email protected] for ?
Its the mailing list for git development.
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
James Purser
http://ksit.dynalias.com
From: "James Purser" <[email protected]>
> On Sun, 2005-05-08 at 19:48, Michael Tokarev wrote:
> > Thomas Glanzmann wrote:
> > > [PATCH] Really *do* nothing in while loop
> > >
> > > Signed-Off-by: Thomas Glanzmann <[email protected]>
> > >
> > > --- a/sha1_file.c
> > > +++ b/sha1_file.c
> > > @@ -335,7 +335,7 @@
> > > stream.next_in = hdr;
> > > stream.avail_in = hdrlen;
> > > while (deflate(&stream, 0) == Z_OK)
> > > - /* nothing */
> > > + /* nothing */;
> > >
> > > /* Then the data itself.. */
> > > stream.next_in = buf;
> >
> > Well, the lack of semicolon is wrong really (and funny).
You guys REALLY do not see the changed semantics here? You are
changing:
while (deflate(&stream, 0) == Z_OK)
stream.next_in = buf;
into
while (deflate(&stream, 0) == Z_OK)
;
/* Then the data itself.. */
stream.next_in = buf;
I suspect the results of that tiny bit of code would be slightly
different, especially if "stream.next_in" is volatile, "buf"
is volatile, or if the assignment to next_in has an effect on
the "deflate" operation.
{^_^}
jdow wrote:
> From: "James Purser" <[email protected]>
>
while (deflate(&stream, 0) == Z_OK)
- /* nothing */
+ /* nothing */;
stream.next_in = buf;
>
> You guys REALLY do not see the changed semantics here? You are
> changing:
> while (deflate(&stream, 0) == Z_OK)
> stream.next_in = buf;
>
> into
>
> while (deflate(&stream, 0) == Z_OK)
> ;
> /* Then the data itself.. */
> stream.next_in = buf;
>
> I suspect the results of that tiny bit of code would be slightly
> different, especially if "stream.next_in" is volatile, "buf"
> is volatile, or if the assignment to next_in has an effect on
> the "deflate" operation.
As I already said, deflate() in this case does only ONE iteration.
stream.avail_in is NOT changed in the loop (except of the deflate()
itself, where it will be set to 0 - provided out buffer have enouth
room). So the whole while loop does only ONE iteration, returning
Z_NEED_DATA or something the next one. So no, the semantics here
(actual semantics) does NOT change.
/mjt
Thomas Glanzmann wrote:
> [PATCH] Really *do* nothing in while loop
>
> Signed-Off-by: Thomas Glanzmann <[email protected]>
>
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -335,7 +335,7 @@
> stream.next_in = hdr;
> stream.avail_in = hdrlen;
> while (deflate(&stream, 0) == Z_OK)
> - /* nothing */
> + /* nothing */;
I am explicitly ignoring the core subject on this thread. This
is a side note, regarding coding style.
I always use a common format for an empty body:
while (deflate(&stream, 0) == Z_OK)
{}
It stands out and positively says what is meant. As such it
makes it much more readable.
I think that CodingStyle should provide guidance for empty
bodies.
--
Eyal Lebedinsky ([email protected]) <http://samba.org/eyal/>
attach .zip as .dat
>>>>> "MT" == Michael Tokarev <[email protected]> writes:
MT> As I already said, deflate() in this case does only ONE iteration.
MT> stream.avail_in is NOT changed in the loop (except of the deflate()
MT> itself, where it will be set to 0 - provided out buffer have enouth
MT> room)....
Just a stupid question, but what happens when we do not have
enough room in the buffer?
On 05.08, Michael Tokarev wrote:
> Thomas Glanzmann wrote:
> > [PATCH] Really *do* nothing in while loop
> >
> > Signed-Off-by: Thomas Glanzmann <[email protected]>
> >
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -335,7 +335,7 @@
> > stream.next_in = hdr;
> > stream.avail_in = hdrlen;
> > while (deflate(&stream, 0) == Z_OK)
> > - /* nothing */
> > + /* nothing */;
> >
> > /* Then the data itself.. */
> > stream.next_in = buf;
>
> Well, the lack of semicolon is wrong really (and funny).
>
> But is the whole while loop needed at all? deflate()
> consumes as much input as it can, producing as much output
> as it can. So without the loop, and without updating the
> buffer pointers ({next,avail}_{in,out}) it will do just
> fine without the loop, and will return something != Z_OK
> on next iteration. If this is to mean to flush output,
> it should be deflate(&stream, Z_FLUSH) or something.
>
This changes the code in the corner case when deflate(...) IS NOT Z_OK
in the first iteration.
Old code: next_in is not assigned if deflate(&stream, 0) != Z_OK
New code: next_is is _always_ assigned
Other point is if old code was wrong...hidden bug ?
--
J.A. Magallon <jamagallon()able!es> \ Software is like sex:
werewolf!able!es \ It's better when it's free
Mandriva Linux release 2006.0 (Cooker) for i586
Linux 2.6.11-jam16 (gcc 4.0.0 (4.0.0-3mdk for Mandriva Linux release 2006.0))