You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Schaefer <jo...@sunstarsys.com> on 2005/08/16 03:54:11 UTC
Re: [PATCH] use arrays in smtpd_request_rec
Garrett Rooney <ro...@electricjellyfish.net> writes:
> Index: smtp_protocol.c
> ===================================================================
> --- smtp_protocol.c (revision 232680)
> +++ smtp_protocol.c (working copy)
[...]
> + for (i = 0; i < sr->extensions->nelts; ++i) {
> + ap_rprintf(r, "%d-%s\r\n", 250, ((char **)sr->extensions->nelts)[i]);
^^^^^
That should be "elts", shouldn't it?
--
Joe Schaefer
Re: [PATCH] use arrays in smtpd_request_rec
Posted by Rian Hunter <ri...@MIT.EDU>.
On Aug 15, 2005, at 10:05 PM, Garrett Rooney wrote:
> Joe Schaefer wrote:
>
>> Garrett Rooney <ro...@electricjellyfish.net> writes:
>>
>>> Index: smtp_protocol.c
>>> ===================================================================
>>> --- smtp_protocol.c (revision 232680)
>>> +++ smtp_protocol.c (working copy)
>>>
>> [...]
>>
>>> + for (i = 0; i < sr->extensions->nelts; ++i) {
>>> + ap_rprintf(r, "%d-%s\r\n", 250, ((char **)sr->extensions-
>>> >nelts)[i]);
>>>
>>
>> ^^^^^
>> That should be "elts", shouldn't it?
>>
>
> Yes indeed, it should. One of the problems with data structures
> that require casting in order to access what you've stored in
> them... The version that Rian committed is slightly different, but
> still has the same problem. I'd post a patch, but it's probably
> faster to fix it by hand than it is to detach a patch from an email
> and apply it.
>
> Over in Subversion-land we have a couple of macros defined for
> dealing with apr arrays, they make it rather difficult to make this
> kind of mistake, and I'd love to see them in the main APR release,
> but the last time it was brought up I believe someone objected to
> their inclusion...
>
> -garrett
>
Fixed. I didn't even catch this because I don't test mod_smtpd with a
plugin that registers extensions. Things like this beckon some kind
of test suite module. Someone can get to this after mod_smtpd is redone.
-rian
Re: [PATCH] use arrays in smtpd_request_rec
Posted by Paul Querna <ch...@force-elite.com>.
Garrett Rooney wrote:
[...]
>>> + for (i = 0; i < sr->extensions->nelts; ++i) {
>>> + ap_rprintf(r, "%d-%s\r\n", 250, ((char
>>> **)sr->extensions->nelts)[i]);
>>
>>
>> ^^^^^
>>
>> That should be "elts", shouldn't it?
>>
>
> Yes indeed, it should. One of the problems with data structures that
> require casting in order to access what you've stored in them... The
> version that Rian committed is slightly different, but still has the
> same problem. I'd post a patch, but it's probably faster to fix it by
> hand than it is to detach a patch from an email and apply it.
>
> Over in Subversion-land we have a couple of macros defined for dealing
> with apr arrays, they make it rather difficult to make this kind of
> mistake, and I'd love to see them in the main APR release, but the last
> time it was brought up I believe someone objected to their inclusion...
I don't object, it sounds like a bloody good idea to me.
-Paul
Re: [PATCH] use arrays in smtpd_request_rec
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Joe Schaefer wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>
>>Index: smtp_protocol.c
>>===================================================================
>>--- smtp_protocol.c (revision 232680)
>>+++ smtp_protocol.c (working copy)
>
>
> [...]
>
>>+ for (i = 0; i < sr->extensions->nelts; ++i) {
>>+ ap_rprintf(r, "%d-%s\r\n", 250, ((char **)sr->extensions->nelts)[i]);
>
> ^^^^^
>
> That should be "elts", shouldn't it?
>
Yes indeed, it should. One of the problems with data structures that
require casting in order to access what you've stored in them... The
version that Rian committed is slightly different, but still has the
same problem. I'd post a patch, but it's probably faster to fix it by
hand than it is to detach a patch from an email and apply it.
Over in Subversion-land we have a couple of macros defined for dealing
with apr arrays, they make it rather difficult to make this kind of
mistake, and I'd love to see them in the main APR release, but the last
time it was brought up I believe someone objected to their inclusion...
-garrett