Forum: NGINX Compilation errors on Ubuntu 8.10 Intrepid Ibex

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
9d8d9b14993d62d615156ff7ae212ac9?d=identicon&s=25 Eric Benson (Guest)
on 2008-11-10 18:48
(Received via mailing list)
Attachment: make.log (3 KB)
Attachment: nginx.diff (4 KB)
Compiling either 0.6.32 or 0.7.20 on a new Ubuntu 8.10 installation with
gcc 4.3.2 causes errors due to a new compiler warning (when combined
with the use of -Werror in CFLAGS). There is now a compiler warning when
the return value of write() is ignored. This causes errors in a number
of placees in nginx. I have included a log showing the compiler output
as well as a patch file showing the changes I made to avoid these
errors.
5640e332954fc0006aea97a155ce0afd?d=identicon&s=25 Igor Sysoev (Guest)
on 2008-11-10 21:43
(Received via mailing list)
Attachment: patch.write_fd (3 KB)
On Mon, Nov 10, 2008 at 09:38:39AM -0800, Eric Benson wrote:

> Compiling either 0.6.32 or 0.7.20 on a new Ubuntu 8.10 installation with gcc 4.3.2 
causes errors due to a new compiler warning (when combined with the use of -Werror in 
CFLAGS). There is now a compiler warning when the return value of write() is ignored. This 
causes errors in a number of placees in nginx. I have included a log showing the compiler 
output as well as a patch file showing the changes I made to avoid these errors.

Thank you.
However, there is more elegant way to suppress this warning without
stub variables:

      (void) write_fd(...)

Could you test the attached patch ?
9d8d9b14993d62d615156ff7ae212ac9?d=identicon&s=25 Eric Benson (Guest)
on 2008-11-10 22:37
(Received via mailing list)
Unfortunately the direct cast to void doesn't suppress the warning. I
tried that first before using the variable assignment (which then
required a cast to void to avoid an unused variable warning!)

Ideally, you should actually check the return value to make sure there
wasn't an error (-1 returned) or an incomplete write (the number of
bytes written is less than the count argument).



----- Original Message ----
From: Igor Sysoev <is@rambler-co.ru>
To: nginx@sysoev.ru
Sent: Monday, November 10, 2008 12:34:37 PM
Subject: Re: Compilation errors on Ubuntu 8.10 Intrepid Ibex

On Mon, Nov 10, 2008 at 09:38:39AM -0800, Eric Benson wrote:

> Compiling either 0.6.32 or 0.7.20 on a new Ubuntu 8.10 installation with gcc 4.3.2 
causes errors due to a new compiler warning (when combined with the use of -Werror in 
CFLAGS). There is now a compiler warning when the return value of write() is ignored. This 
causes errors in a number of placees in nginx. I have included a log showing the compiler 
output as well as a patch file showing the changes I made to avoid these errors.

Thank you.
However, there is more elegant way to suppress this warning without
stub variables:

      (void) write_fd(...)

Could you test the attached patch ?
A8108a0961c6087c43cda32c8616dcba?d=identicon&s=25 Maxim Dounin (Guest)
on 2008-11-10 22:41
(Received via mailing list)
Hello!

On Mon, Nov 10, 2008 at 11:34:37PM +0300, Igor Sysoev wrote:

> Could you test the attached patch ?
No, warn_unused_result warning can't be avoided by casting to
void:

$ cat unused.c
int foo() __attribute__((warn_unused_result));

int
foo()
{
        return 1;
}

int
main()
{
        (void) foo();

        return 0;
}
$ gcc unused.c
unused.c: In function 'main':
unused.c:12: warning: ignoring return value of 'foo', declared with
attribute warn_unused_result

More details here:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509

However I don't think it's a good idea to use stub variables.
Probably something like

    if (ngx_write_fd(...) == NGX_FILE_ERROR) {
        /* void */
    }

would be better.

Maxim Dounin

p.s. Actually I think that it's glibc's bug.  But we probably have
to live with it, at least for some time.
5640e332954fc0006aea97a155ce0afd?d=identicon&s=25 Igor Sysoev (Guest)
on 2008-11-10 22:53
(Received via mailing list)
On Tue, Nov 11, 2008 at 12:35:40AM +0300, Maxim Dounin wrote:

> > stub variables:
>
>
> However I don't think it's a good idea to use stub variables.
> p.s. Actually I think that it's glibc's bug.  But we probably have
> to live with it, at least for some time.

I believe this is gcc bug because I explictly says that I do not need
the function result using (void) case.

Dummy result checking as well as stub variable are security/reliabilty
profanation.  The (void) cast should be compact way to do the same.
9d8d9b14993d62d615156ff7ae212ac9?d=identicon&s=25 Eric Benson (Guest)
on 2008-11-10 23:37
(Received via mailing list)
Whether it is gcc's bug because cast to void does not suppress the
compiler warning, or it is glibc's bug because it declares write() with
attribute warn_unused_result, you are unlikely to see a change from
either in the near future. The best course is to take it as an
opportunity to do some paranoid error checking in these cases. In most
cases, there is some better action to be taken when write() fails. In
the worst case, such as trying to write a log entry to a full file
system, it is better to exit() with non-zero status than to silently
continue running. This is why the attribute was added to the declaration
of write(). Most projects do not use -Werror, so they only see added
compiler output instead of termination of the build process. I agree
that it seems like an awfully big change that might adversely affect
many mature software projects. However, there is virtually no case when
it is correct to ignore an exceptional condition in write().



----- Original Message ----
From: Igor Sysoev <is@rambler-co.ru>
To: nginx@sysoev.ru
Sent: Monday, November 10, 2008 1:43:54 PM
Subject: Re: Compilation errors on Ubuntu 8.10 Intrepid Ibex

On Tue, Nov 11, 2008 at 12:35:40AM +0300, Maxim Dounin wrote:

> > stub variables:
>
>
> However I don't think it's a good idea to use stub variables.
> p.s. Actually I think that it's glibc's bug.  But we probably have
> to live with it, at least for some time.

I believe this is gcc bug because I explictly says that I do not need
the function result using (void) case.

Dummy result checking as well as stub variable are security/reliabilty
profanation.  The (void) cast should be compact way to do the same.
A8108a0961c6087c43cda32c8616dcba?d=identicon&s=25 Maxim Dounin (Guest)
on 2008-11-11 00:57
(Received via mailing list)
Hello!

On Tue, Nov 11, 2008 at 12:43:54AM +0300, Igor Sysoev wrote:

> On Tue, Nov 11, 2008 at 12:35:40AM +0300, Maxim Dounin wrote:

[...]

> >
> > would be better.
> >
> > Maxim Dounin
> >
> > p.s. Actually I think that it's glibc's bug.  But we probably have
> > to live with it, at least for some time.
>
> I believe this is gcc bug because I explictly says that I do not need
> the function result using (void) case.

It can't be gcc bug - warn_unused_result was explicitly coded to
produce warning unavoidable by (void) cast.  I agree that it's
a misfeature though.

Anyway, there are many handwaving about this at the url I posted
(and in other places as well).  I don't think we want to repeat it
here.  :)

> Dummy result checking as well as stub variable are security/reliabilty
> profanation.

With [dummy] result checking we have some good place where we can
write reason why we can't do anything with the result,
so it looks better for me than stub variable.  Of course both
aren't ideal.

> The (void) cast should be compact way to do the same.

It should, but it isn't.  The question now is how to live with it
with minimal impact on decent systems.  Probably the best solution
would be just to omit -Werror on such systems.

Maxim Dounin
A8108a0961c6087c43cda32c8616dcba?d=identicon&s=25 Maxim Dounin (Guest)
on 2008-11-11 01:36
(Received via mailing list)
Hello!

[... warn_unused_result discussion skipped ...]

What about attached patch instead?  Actually, there is only 1
place where we really can't do anything - write() while logging
error.  In other places sensible behaviour is possible.

Maxim Dounin
5640e332954fc0006aea97a155ce0afd?d=identicon&s=25 Igor Sysoev (Guest)
on 2008-11-11 07:46
(Received via mailing list)
Attachment: patch.write_fd1 (3 KB)
On Mon, Nov 10, 2008 at 02:27:43PM -0800, Eric Benson wrote:

> Whether it is gcc's bug because cast to void does not suppress the compiler warning, or 
it is glibc's bug because it declares write() with attribute warn_unused_result, you are 
unlikely to see a change from either in the near future. The best course is to take it as 
an opportunity to do some paranoid error checking in these cases. In most cases, there is 
some better action to be taken when write() fails. In the worst case, such as trying to 
write a log entry to a full file system, it is better to exit() with non-zero status than 
to silently continue running. This is why the attribute was added to the declaration of 
write(). Most projects do not use -Werror, so they only see added compiler output instead 
of termination of the build process. I agree that it seems like an awfully big change that 
might adversely affect many mature software projects. However, there is virtually no case 
when it is correct to ignore an exceptional condition in write().

I believe, that many prefer that server continues to serve site even its
log
filesystem is full. So there is no way how to handle write() error while
logging another error except to ignore it.

Here is the new preliminary patch to test work-around.
9d8d9b14993d62d615156ff7ae212ac9?d=identicon&s=25 Eric Benson (Guest)
on 2008-11-11 08:32
(Received via mailing list)
Yes, this works. In fact, after you change it from a macro to a static
function you don't need a cast to void. So, changing the header file is
sufficient.

I can see the argument for continuing to serve pages even if the log
filesystem is full. I have experience with the opposite case. It all
depends on how critical the logs are for a business. Having been at
Amazon.com in 1996 and 1997 I can tell you that there were times when we
were able to reconstruct some customer transactions using web server
logs after databases were damaged. If it were up to me, I would at least
want this to be a configuration option.



----- Original Message ----
From: Igor Sysoev <is@rambler-co.ru>
To: nginx@sysoev.ru
Sent: Monday, November 10, 2008 10:35:36 PM
Subject: Re: Compilation errors on Ubuntu 8.10 Intrepid Ibex

On Mon, Nov 10, 2008 at 02:27:43PM -0800, Eric Benson wrote:

> Whether it is gcc's bug because cast to void does not suppress the compiler warning, or 
it is glibc's bug because it declares write() with attribute warn_unused_result, you are 
unlikely to see a change from either in the near future. The best course is to take it as 
an opportunity to do some paranoid error checking in these cases. In most cases, there is 
some better action to be taken when write() fails. In the worst case, such as trying to 
write a log entry to a full file system, it is better to exit() with non-zero status than 
to silently continue running. This is why the attribute was added to the declaration of 
write(). Most projects do not use -Werror, so they only see added compiler output instead 
of termination of the build process. I agree that it seems like an awfully big change that 
might adversely affect many mature software projects. However, there is virtually no case 
when it is correct to ignore an exceptional condition in write().

I believe, that many prefer that server continues to serve site even its
log
filesystem is full. So there is no way how to handle write() error while
logging another error except to ignore it.

Here is the new preliminary patch to test work-around.
F5a6ed477b109fe6acc11a5a8f87e7e8?d=identicon&s=25 mike (Guest)
on 2008-11-11 08:39
(Received via mailing list)
On Mon, Nov 10, 2008 at 11:25 PM, Eric Benson <eric_a_benson@yahoo.com>
wrote:

> I can see the argument for continuing to serve pages even if the log filesystem is full. 
I have experience with the opposite case. It all depends on how critical the logs are for 
a business. Having been at Amazon.com in 1996 and 1997 I can tell you that there were 
times when we were able to reconstruct some customer transactions using web server logs 
after databases were damaged. If it were up to me, I would at least want this to be a 
configuration option.

I'd be all for keeping the lights on...

IMHO, if the log gets full, the server shouldn't be down. My clients
don't care about logs. They'd rather have their site up. There should
be a separate monitoring and notification system in place for low
space threshholds...
This topic is locked and can not be replied to.