Mailing List Archive

Re: svn commit: r1901500 - in /httpd/httpd/trunk: include/http_protocol.h server/protocol.c
On Wed, 1 Jun 2022 at 15:34, <covener@apache.org> wrote:
>
> Author: covener
> Date: Wed Jun 1 12:33:53 2022
> New Revision: 1901500
>
> URL: http://svn.apache.org/viewvc?rev=1901500&view=rev
> Log:
> handle large writes in ap_rputs
>
> Modified:
> httpd/httpd/trunk/include/http_protocol.h
> httpd/httpd/trunk/server/protocol.c
>
> Modified: httpd/httpd/trunk/include/http_protocol.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_protocol.h?rev=1901500&r1=1901499&r2=1901500&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/http_protocol.h (original)
> +++ httpd/httpd/trunk/include/http_protocol.h Wed Jun 1 12:33:53 2022
> @@ -501,7 +501,27 @@ AP_DECLARE(int) ap_rwrite(const void *bu
> */
> static APR_INLINE int ap_rputs(const char *str, request_rec *r)
> {
> - return ap_rwrite(str, (int)strlen(str), r);
> + apr_size_t len;
> +
> + len = strlen(str);
> +
> + for (;;) {
> + if (len <= INT_MAX) {
> + return ap_rwrite(str, (int)len, r);
> + }
> + else {
> + int rc;
> +
> + rc = ap_rwrite(str, INT_MAX, r);
> + if (rc < 0) {
> + return rc;
> + }
> + else {
> + str += INT_MAX;
> + len -= INT_MAX;
> + }
> + }
> + }
After this change apr_rputs() doesn't return value.

--
Ivan Zhakov
Re: svn commit: r1901500 - in /httpd/httpd/trunk: include/http_protocol.h server/protocol.c [ In reply to ]
On Wed, Jun 8, 2022 at 11:10 AM Ivan Zhakov <ivan@visualsvn.com> wrote:
>
> On Wed, 1 Jun 2022 at 15:34, <covener@apache.org> wrote:
> >
> > Author: covener
> > Date: Wed Jun 1 12:33:53 2022
> > New Revision: 1901500
> >
> > URL: http://svn.apache.org/viewvc?rev=1901500&view=rev
> > Log:
> > handle large writes in ap_rputs
> >
> > Modified:
> > httpd/httpd/trunk/include/http_protocol.h
> > httpd/httpd/trunk/server/protocol.c
> >
> > Modified: httpd/httpd/trunk/include/http_protocol.h
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_protocol.h?rev=1901500&r1=1901499&r2=1901500&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/include/http_protocol.h (original)
> > +++ httpd/httpd/trunk/include/http_protocol.h Wed Jun 1 12:33:53 2022
> > @@ -501,7 +501,27 @@ AP_DECLARE(int) ap_rwrite(const void *bu
> > */
> > static APR_INLINE int ap_rputs(const char *str, request_rec *r)
> > {
> > - return ap_rwrite(str, (int)strlen(str), r);
> > + apr_size_t len;
> > +
> > + len = strlen(str);
> > +
> > + for (;;) {
> > + if (len <= INT_MAX) {
> > + return ap_rwrite(str, (int)len, r);
> > + }
> > + else {
> > + int rc;
> > +
> > + rc = ap_rwrite(str, INT_MAX, r);
> > + if (rc < 0) {
> > + return rc;
> > + }
> > + else {
> > + str += INT_MAX;
> > + len -= INT_MAX;
> > + }
> > + }
> > + }
> After this change apr_rputs() doesn't return value.

Since there is no break, it seems to propagate ap_write return value no?
Re: svn commit: r1901500 - in /httpd/httpd/trunk: include/http_protocol.h server/protocol.c [ In reply to ]
Yes, I see now. But it will be an incorrect value in case of a string
larger than INT_MAX. Not a big issue, but IMHO strings larger than
INT_MAX also are not big issue.

On Wed, 8 Jun 2022 at 18:26, Eric Covener <covener@gmail.com> wrote:
>
> On Wed, Jun 8, 2022 at 11:10 AM Ivan Zhakov <ivan@visualsvn.com> wrote:
> >
> > On Wed, 1 Jun 2022 at 15:34, <covener@apache.org> wrote:
> > >
> > > Author: covener
> > > Date: Wed Jun 1 12:33:53 2022
> > > New Revision: 1901500
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1901500&view=rev
> > > Log:
> > > handle large writes in ap_rputs
> > >
> > > Modified:
> > > httpd/httpd/trunk/include/http_protocol.h
> > > httpd/httpd/trunk/server/protocol.c
> > >
> > > Modified: httpd/httpd/trunk/include/http_protocol.h
> > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_protocol.h?rev=1901500&r1=1901499&r2=1901500&view=diff
> > > ==============================================================================
> > > --- httpd/httpd/trunk/include/http_protocol.h (original)
> > > +++ httpd/httpd/trunk/include/http_protocol.h Wed Jun 1 12:33:53 2022
> > > @@ -501,7 +501,27 @@ AP_DECLARE(int) ap_rwrite(const void *bu
> > > */
> > > static APR_INLINE int ap_rputs(const char *str, request_rec *r)
> > > {
> > > - return ap_rwrite(str, (int)strlen(str), r);
> > > + apr_size_t len;
> > > +
> > > + len = strlen(str);
> > > +
> > > + for (;;) {
> > > + if (len <= INT_MAX) {
> > > + return ap_rwrite(str, (int)len, r);
> > > + }
> > > + else {
> > > + int rc;
> > > +
> > > + rc = ap_rwrite(str, INT_MAX, r);
> > > + if (rc < 0) {
> > > + return rc;
> > > + }
> > > + else {
> > > + str += INT_MAX;
> > > + len -= INT_MAX;
> > > + }
> > > + }
> > > + }
> > After this change apr_rputs() doesn't return value.
>
> Since there is no break, it seems to propagate ap_write return value no?



--
Ivan Zhakov
Re: svn commit: r1901500 - in /httpd/httpd/trunk: include/http_protocol.h server/protocol.c [ In reply to ]
On 6/8/22 5:43 PM, Ivan Zhakov wrote:
> Yes, I see now. But it will be an incorrect value in case of a string
> larger than INT_MAX. Not a big issue, but IMHO strings larger than
> INT_MAX also are not big issue.

You are correct that the value will be incorrect in case of a string larger than INT_MAX, but as ap_rputs can only return an int
and as we cannot change this without breaking the API/ABI it looked like the best option to return just the value of the last call
to ap_rwrite.

One thing that comes to my mind now: As ap_rputs is static APR_INLINE in include/http_protocol.h I guess that third party modules
that use it need to recompile to pick this up, correct?

Regards

RĂ¼diger