You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by mi...@apache.org on 2011/11/28 23:58:11 UTC
svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c
dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c
util-misc/apr_thread_pool.c
Author: minfrin
Date: Mon Nov 28 22:58:09 2011
New Revision: 1207680
URL: http://svn.apache.org/viewvc?rev=1207680&view=rev
Log:
Remove variables that we assign but never read.
Modified:
apr/apr/trunk/crypto/apr_crypto_nss.c
apr/apr/trunk/dbd/apr_dbd_sqlite3.c
apr/apr/trunk/dbm/apr_dbm_sdbm.c
apr/apr/trunk/test/testreslist.c
apr/apr/trunk/util-misc/apr_thread_pool.c
Modified: apr/apr/trunk/crypto/apr_crypto_nss.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/crypto/apr_crypto_nss.c?rev=1207680&r1=1207679&r2=1207680&view=diff
==============================================================================
--- apr/apr/trunk/crypto/apr_crypto_nss.c (original)
+++ apr/apr/trunk/crypto/apr_crypto_nss.c Mon Nov 28 22:58:09 2011
@@ -114,7 +114,6 @@ static apr_status_t crypto_shutdown(void
static apr_status_t crypto_shutdown_helper(void *data)
{
- apr_pool_t *pool = (apr_pool_t *) data;
return crypto_shutdown();
}
Modified: apr/apr/trunk/dbd/apr_dbd_sqlite3.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbd/apr_dbd_sqlite3.c?rev=1207680&r1=1207679&r2=1207680&view=diff
==============================================================================
--- apr/apr/trunk/dbd/apr_dbd_sqlite3.c (original)
+++ apr/apr/trunk/dbd/apr_dbd_sqlite3.c Mon Nov 28 22:58:09 2011
@@ -118,7 +118,6 @@ static int dbd_sqlite3_select_internal(a
}
} else if (ret == SQLITE_ROW) {
int length;
- apr_dbd_column_t *col;
row = apr_palloc(pool, sizeof(apr_dbd_row_t));
row->res = *results;
increment = sizeof(apr_dbd_column_t *);
@@ -157,7 +156,6 @@ static int dbd_sqlite3_select_internal(a
case SQLITE_NULL:
break;
}
- col = row->columns[i];
}
row->rownum = num_tuples++;
row->next_row = 0;
Modified: apr/apr/trunk/dbm/apr_dbm_sdbm.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbm/apr_dbm_sdbm.c?rev=1207680&r1=1207679&r2=1207680&view=diff
==============================================================================
--- apr/apr/trunk/dbm/apr_dbm_sdbm.c (original)
+++ apr/apr/trunk/dbm/apr_dbm_sdbm.c Mon Nov 28 22:58:09 2011
@@ -184,10 +184,9 @@ static apr_status_t vt_sdbm_firstkey(apr
static apr_status_t vt_sdbm_nextkey(apr_dbm_t *dbm, apr_datum_t *pkey)
{
- apr_status_t rv;
apr_sdbm_datum_t rd;
- rv = apr_sdbm_nextkey(dbm->file, &rd);
+ apr_sdbm_nextkey(dbm->file, &rd);
pkey->dptr = rd.dptr;
pkey->dsize = rd.dsize;
Modified: apr/apr/trunk/test/testreslist.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/test/testreslist.c?rev=1207680&r1=1207679&r2=1207680&view=diff
==============================================================================
--- apr/apr/trunk/test/testreslist.c (original)
+++ apr/apr/trunk/test/testreslist.c Mon Nov 28 22:58:09 2011
@@ -142,7 +142,6 @@ static void test_timeout(abts_case *tc,
{
apr_status_t rv;
my_resource_t *resources[RESLIST_HMAX];
- my_resource_t *res;
void *vp;
int i;
@@ -163,8 +162,6 @@ static void test_timeout(abts_case *tc,
rv = apr_reslist_acquire(rl, &vp);
ABTS_TRUE(tc, APR_STATUS_IS_TIMEUP(rv));
- res = vp;
-
/* release the resources; otherwise the destroy operation
* will blow
*/
Modified: apr/apr/trunk/util-misc/apr_thread_pool.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/util-misc/apr_thread_pool.c?rev=1207680&r1=1207679&r2=1207680&view=diff
==============================================================================
--- apr/apr/trunk/util-misc/apr_thread_pool.c (original)
+++ apr/apr/trunk/util-misc/apr_thread_pool.c Mon Nov 28 22:58:09 2011
@@ -237,7 +237,6 @@ static struct apr_thread_list_elt *elt_n
*/
static void *APR_THREAD_FUNC thread_pool_func(apr_thread_t * t, void *param)
{
- apr_status_t rv = APR_SUCCESS;
apr_thread_pool_t *me = param;
apr_thread_pool_task_t *task = NULL;
apr_interval_time_t wait;
@@ -313,10 +312,10 @@ static void *APR_THREAD_FUNC thread_pool
wait = -1;
if (wait >= 0) {
- rv = apr_thread_cond_timedwait(me->cond, me->lock, wait);
+ apr_thread_cond_timedwait(me->cond, me->lock, wait);
}
else {
- rv = apr_thread_cond_wait(me->cond, me->lock);
+ apr_thread_cond_wait(me->cond, me->lock);
}
}
Re: svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c util-misc/apr_thread_pool.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 29 November 2011, William A. Rowe Jr. wrote:
> On 11/28/2011 5:32 PM, William A. Rowe Jr. wrote:
> > On 11/28/2011 5:29 PM, Graham Leggett wrote:
> >> On 29 Nov 2011, at 01:21, William A. Rowe Jr. wrote:
> >>>> - rv = apr_sdbm_nextkey(dbm->file,&rd);
> >>>> + apr_sdbm_nextkey(dbm->file,&rd);
> >>>>
> >>>> pkey->dptr = rd.dptr;
> >>>> pkey->dsize = rd.dsize;
> >>
> >> apr-trunk contains the following explanation for this, I
> >> understand it's intended (sf?):
> >>
> >> /*
> >> * XXX: This discards any error but apr_sdbm_nextkey currently
> >> returns * XXX: an error for the last key
> >> */
> >
> > Interesting. Good if that's the only case; it still seems odd ;-)
The error handling of apr_sdbm_nextkey is broken, but I don't remember
the details.
> FWIW, isn't this invalid on some builds?
>
> Don't you need to cast the ignored function rv to null?
>
> Seems to be trading your warning for someone elses' warning.
>
> E.g.
>
> (void)apr_sdbm_nextkey(dbm->file, &rd);
IMHO this should only be necessary if apr_sdbm_nextkey() had attribute
warn_unused_result.
Re: svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c util-misc/apr_thread_pool.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 29 Nov 2011, at 01:35, William A. Rowe Jr. wrote:
> FWIW, isn't this invalid on some builds?
>
> Don't you need to cast the ignored function rv to null?
>
> Seems to be trading your warning for someone elses' warning.
>
> E.g.
>
> (void)apr_sdbm_nextkey(dbm->file, &rd);
This definitely seems sensible. Just tried the build cast to void, and it seemed to work without a warning (rpmbuild from Fedora Core 17). Committed in r1207704.
Regards,
Graham
--
Re: svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c
dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c util-misc/apr_thread_pool.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 11/28/2011 5:32 PM, William A. Rowe Jr. wrote:
> On 11/28/2011 5:29 PM, Graham Leggett wrote:
>> On 29 Nov 2011, at 01:21, William A. Rowe Jr. wrote:
>>
>>>> - rv = apr_sdbm_nextkey(dbm->file,&rd);
>>>> + apr_sdbm_nextkey(dbm->file,&rd);
>>>>
>>>> pkey->dptr = rd.dptr;
>>>> pkey->dsize = rd.dsize;
>>
>> apr-trunk contains the following explanation for this, I understand it's intended (sf?):
>>
>> /*
>> * XXX: This discards any error but apr_sdbm_nextkey currently returns
>> * XXX: an error for the last key
>> */
>
> Interesting. Good if that's the only case; it still seems odd ;-)
FWIW, isn't this invalid on some builds?
Don't you need to cast the ignored function rv to null?
Seems to be trading your warning for someone elses' warning.
E.g.
(void)apr_sdbm_nextkey(dbm->file, &rd);
Re: svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c
dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c util-misc/apr_thread_pool.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 11/28/2011 5:29 PM, Graham Leggett wrote:
> On 29 Nov 2011, at 01:21, William A. Rowe Jr. wrote:
>
>>> - rv = apr_sdbm_nextkey(dbm->file,&rd);
>>> + apr_sdbm_nextkey(dbm->file,&rd);
>>>
>>> pkey->dptr = rd.dptr;
>>> pkey->dsize = rd.dsize;
>
> apr-trunk contains the following explanation for this, I understand it's intended (sf?):
>
> /*
> * XXX: This discards any error but apr_sdbm_nextkey currently returns
> * XXX: an error for the last key
> */
Interesting. Good if that's the only case; it still seems odd ;-)
>>> - rv = apr_thread_cond_wait(me->cond, me->lock);
>>> + apr_thread_cond_wait(me->cond, me->lock);
>>> }
>>> }
>
> Again, not sure what the correct behaviour is here. The function returns void, so the caller isn't expecting an error.
Ah, you are correct, this is not a trylock.
Re: svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c util-misc/apr_thread_pool.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 29 Nov 2011, at 01:21, William A. Rowe Jr. wrote:
>> Modified: apr/apr/trunk/dbm/apr_dbm_sdbm.c
>> URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbm/apr_dbm_sdbm.c?rev=1207680&r1=1207679&r2=1207680&view=diff
>> ==============================================================================
>> --- apr/apr/trunk/dbm/apr_dbm_sdbm.c (original)
>> +++ apr/apr/trunk/dbm/apr_dbm_sdbm.c Mon Nov 28 22:58:09 2011
>> @@ -184,10 +184,9 @@ static apr_status_t vt_sdbm_firstkey(apr
>>
>> static apr_status_t vt_sdbm_nextkey(apr_dbm_t *dbm, apr_datum_t *pkey)
>> {
>> - apr_status_t rv;
>> apr_sdbm_datum_t rd;
>>
>> - rv = apr_sdbm_nextkey(dbm->file,&rd);
>> + apr_sdbm_nextkey(dbm->file,&rd);
>>
>> pkey->dptr = rd.dptr;
>> pkey->dsize = rd.dsize;
apr-trunk contains the following explanation for this, I understand it's intended (sf?):
/*
* XXX: This discards any error but apr_sdbm_nextkey currently returns
* XXX: an error for the last key
*/
>> Modified: apr/apr/trunk/util-misc/apr_thread_pool.c
>> URL: http://svn.apache.org/viewvc/apr/apr/trunk/util-misc/apr_thread_pool.c?rev=1207680&r1=1207679&r2=1207680&view=diff
>> ==============================================================================
>> --- apr/apr/trunk/util-misc/apr_thread_pool.c (original)
>> +++ apr/apr/trunk/util-misc/apr_thread_pool.c Mon Nov 28 22:58:09 2011
>> @@ -237,7 +237,6 @@ static struct apr_thread_list_elt *elt_n
>> */
>> static void *APR_THREAD_FUNC thread_pool_func(apr_thread_t * t, void *param)
>> {
>> - apr_status_t rv = APR_SUCCESS;
>> apr_thread_pool_t *me = param;
>> apr_thread_pool_task_t *task = NULL;
>> apr_interval_time_t wait;
>> @@ -313,10 +312,10 @@ static void *APR_THREAD_FUNC thread_pool
>> wait = -1;
>>
>> if (wait>= 0) {
>> - rv = apr_thread_cond_timedwait(me->cond, me->lock, wait);
>> + apr_thread_cond_timedwait(me->cond, me->lock, wait);
>> }
>> else {
>> - rv = apr_thread_cond_wait(me->cond, me->lock);
>> + apr_thread_cond_wait(me->cond, me->lock);
>> }
>> }
Again, not sure what the correct behaviour is here. The function returns void, so the caller isn't expecting an error.
Regards,
Graham
--
Re: svn commit: r1207680 - in /apr/apr/trunk: crypto/apr_crypto_nss.c
dbd/apr_dbd_sqlite3.c dbm/apr_dbm_sdbm.c test/testreslist.c util-misc/apr_thread_pool.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 11/28/2011 4:58 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Mon Nov 28 22:58:09 2011
> New Revision: 1207680
>
> URL: http://svn.apache.org/viewvc?rev=1207680&view=rev
> Log:
> Remove variables that we assign but never read.
>
> Modified:
> apr/apr/trunk/crypto/apr_crypto_nss.c
> apr/apr/trunk/dbd/apr_dbd_sqlite3.c
> apr/apr/trunk/dbm/apr_dbm_sdbm.c
> apr/apr/trunk/test/testreslist.c
> apr/apr/trunk/util-misc/apr_thread_pool.c
> Modified: apr/apr/trunk/dbm/apr_dbm_sdbm.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbm/apr_dbm_sdbm.c?rev=1207680&r1=1207679&r2=1207680&view=diff
> ==============================================================================
> --- apr/apr/trunk/dbm/apr_dbm_sdbm.c (original)
> +++ apr/apr/trunk/dbm/apr_dbm_sdbm.c Mon Nov 28 22:58:09 2011
> @@ -184,10 +184,9 @@ static apr_status_t vt_sdbm_firstkey(apr
>
> static apr_status_t vt_sdbm_nextkey(apr_dbm_t *dbm, apr_datum_t *pkey)
> {
> - apr_status_t rv;
> apr_sdbm_datum_t rd;
>
> - rv = apr_sdbm_nextkey(dbm->file,&rd);
> + apr_sdbm_nextkey(dbm->file,&rd);
>
> pkey->dptr = rd.dptr;
> pkey->dsize = rd.dsize;
>
> Modified: apr/apr/trunk/util-misc/apr_thread_pool.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/util-misc/apr_thread_pool.c?rev=1207680&r1=1207679&r2=1207680&view=diff
> ==============================================================================
> --- apr/apr/trunk/util-misc/apr_thread_pool.c (original)
> +++ apr/apr/trunk/util-misc/apr_thread_pool.c Mon Nov 28 22:58:09 2011
> @@ -237,7 +237,6 @@ static struct apr_thread_list_elt *elt_n
> */
> static void *APR_THREAD_FUNC thread_pool_func(apr_thread_t * t, void *param)
> {
> - apr_status_t rv = APR_SUCCESS;
> apr_thread_pool_t *me = param;
> apr_thread_pool_task_t *task = NULL;
> apr_interval_time_t wait;
> @@ -313,10 +312,10 @@ static void *APR_THREAD_FUNC thread_pool
> wait = -1;
>
> if (wait>= 0) {
> - rv = apr_thread_cond_timedwait(me->cond, me->lock, wait);
> + apr_thread_cond_timedwait(me->cond, me->lock, wait);
> }
> else {
> - rv = apr_thread_cond_wait(me->cond, me->lock);
> + apr_thread_cond_wait(me->cond, me->lock);
> }
> }
These two would appear to be bugs, which your commit conceals.