---------- Forwarded message ----------
From: Howard Chu <h...@symas.com>
Message-ID: <b2110225-7a22-49c0-1d69-f0de55f1f...@symas.com>
Date: Tue, 18 Dec 2018 23:02:49 +0000
Subject: Re: (ITS#8890) Bug fix patches unbreaking the build on x32 systems

t...@debian.org wrote:
> Full_Name: mirabilos
> Version: 2.4.46
> OS: Debian
> URL: 
> Submission from: (NULL) (2a01:4f8:150:946c::42:3)
> 
> 
> I’ve just managed to unbreak the build of openldap on x32
> (basically amd64ilp32: a 64-bit x86 architecture using 32-bit
> “long” and pointers) by fixing a couple of simple GCC warnings.
> 
> Original bugreport: https://bugs.debian.org/905237
> 
> Please find the attached patches (pretty machinal, so not
> copyright-relevant) and include them in your next release.
> Thanks!
> 
> --- a/clients/tools/common.c
> +++ b/clients/tools/common.c
> @@ -2326,7 +2326,7 @@ void tool_print_ctrls(
>               /* known controls */
>               for ( j = 0; tool_ctrl_response[j].oid != NULL; j++ ) {
>                       if ( strcmp( tool_ctrl_response[j].oid, 
> ctrls[i]->ldctl_oid ) == 0 ) {
> -                             if ( !tool_ctrl_response[j].mask & tool_type ) {
> +                             if ( !(tool_ctrl_response[j].mask & tool_type) 
> ) {
>                                       /* this control should not appear
>                                        * with this tool; warning? */
>                               }

This is patching an if statement with an empty body - it is a pure no-op that
the compiler will optimize away. There's no point in changing this other than
to shut up stupid static analyzer tools. But since you're not the first person
to run a stupid static analyzer tool and point this out, I've merged this "fix."

The following 3 patches are valid and have been merged.
> --- a/servers/slapd/backend.c
> +++ b/servers/slapd/backend.c
> @@ -1500,7 +1500,7 @@ fe_acl_group(
>                                        * or if filter parsing fails.
>                                        * In the latter case,
>                                        * we should give up. */
> -                                     if ( ludp->lud_filter != NULL && 
> ludp->lud_filter != '\0') {
> +                                     if ( ludp->lud_filter != NULL && 
> ludp->lud_filter[0] != '\0') {
>                                               filter = str2filter_x( op, 
> ludp->lud_filter );
>                                               if ( filter == NULL ) {
>                                                       /* give up... */
> --- a/servers/slapd/overlays/constraint.c
> +++ b/servers/slapd/overlays/constraint.c
> @@ -446,7 +446,7 @@ constraint_cf_gen( ConfigArgs *c )
>                                               }
>  
>                                               if ( ap.restrict_lud->lud_attrs 
> != NULL ) {
> -                                                     if ( 
> ap.restrict_lud->lud_attrs[0] != '\0' ) {
> +                                                     if ( 
> ap.restrict_lud->lud_attrs[0] != NULL ) {
>                                                               snprintf( 
> c->cr_msg, sizeof( c->cr_msg ),
>                                                                       "%s %s: 
> attrs not allowed in restrict URI %s\n",
>                                                                       
> c->argv[0], c->argv[1], arg);
> --- a/servers/slapd/syntax.c
> +++ b/servers/slapd/syntax.c
> @@ -219,8 +219,8 @@ syn_add(
>                       }
>  
>                       assert( (*lsei)->lsei_values != NULL );
> -                     if ( (*lsei)->lsei_values[0] == '\0'
> -                             || (*lsei)->lsei_values[1] != '\0' )
> +                     if ( (*lsei)->lsei_values[0] == NULL
> +                             || (*lsei)->lsei_values[1] != NULL )
>                       {
>                               Debug( LDAP_DEBUG_ANY, "syn_add(%s): exactly 
> one substitute syntax must be
> present\n",
>                                       ssyn->ssyn_syn.syn_oid, 0, 0 );

This chunk no longer exists in the repo, so it's been omitted.
> --- a/tests/progs/slapd-addel.c
> +++ b/tests/progs/slapd-addel.c
> @@ -173,7 +173,7 @@ main( int argc, char **argv )
>  
>       }
>  
> -     if (( attrs == NULL ) || ( *attrs == '\0' )) {
> +     if (( attrs == NULL ) || ( *attrs == NULL )) {
>  
>               fprintf( stderr, "%s: invalid attrs in file \"%s\".\n",
>                               argv[0], filename );

This patch is trying to cast to a long long and print the result into a buffer
that is explicitly sized for a long. That's clearly a bug. The remaining format
patches for debug statements will have no impact on the correctness of
execution or output. This and all of the following patches are being rejected.
> --- a/contrib/slapd-modules/smbk5pwd/smbk5pwd.c
> +++ b/contrib/slapd-modules/smbk5pwd/smbk5pwd.c
> @@ -605,7 +605,7 @@ static int smbk5pwd_exop_passwd(
>               keys[0].bv_val = ch_malloc( LDAP_PVT_INTTYPE_CHARS(long) );
>               keys[0].bv_len = snprintf(keys[0].bv_val,
>                       LDAP_PVT_INTTYPE_CHARS(long),
> -                     "%ld", slap_get_time());
> +                     "%lld", (long long)slap_get_time());
>               BER_BVZERO( &keys[1] );
>               
>               ml->sml_desc = ad_sambaPwdLastSet;
> @@ -627,7 +627,7 @@ static int smbk5pwd_exop_passwd(
>                       keys[0].bv_val = ch_malloc( 
> LDAP_PVT_INTTYPE_CHARS(long) );
>                       keys[0].bv_len = snprintf(keys[0].bv_val,
>                                       LDAP_PVT_INTTYPE_CHARS(long),
> -                                     "%ld", slap_get_time() + 
> pi->smb_must_change);
> +                                     "%lld", (long long)slap_get_time() + 
> (long long)pi->smb_must_change);
>                       BER_BVZERO( &keys[1] );
>  
>                       ml->sml_desc = ad_sambaPwdMustChange;
> @@ -650,7 +650,7 @@ static int smbk5pwd_exop_passwd(
>                       keys[0].bv_val = ch_malloc( 
> LDAP_PVT_INTTYPE_CHARS(long) );
>                       keys[0].bv_len = snprintf(keys[0].bv_val,
>                                       LDAP_PVT_INTTYPE_CHARS(long),
> -                                     "%ld", slap_get_time() + 
> pi->smb_can_change);
> +                                     "%lld", (long long)slap_get_time() + 
> (long long)pi->smb_can_change);
>                       BER_BVZERO( &keys[1] );
>  
>                       ml->sml_desc = ad_sambaPwdCanChange;
> --- a/libraries/libldap/os-ip.c
> +++ b/libraries/libldap/os-ip.c
> @@ -282,8 +282,8 @@ ldap_int_poll(
>       int             rc;
>               
>  
> -     osip_debug(ld, "ldap_int_poll: fd: %d tm: %ld\n",
> -             s, tvp ? tvp->tv_sec : -1L, 0);
> +     osip_debug(ld, "ldap_int_poll: fd: %d tm: %lld\n",
> +             s, tvp ? (long long)tvp->tv_sec : -1LL, 0);
>  
>  #ifdef HAVE_POLL
>       {
> @@ -432,8 +432,8 @@ ldap_pvt_connect(LDAP *ld, ber_socket_t
>               opt_tv = &tv;
>       }
>  
> -     osip_debug(ld, "ldap_pvt_connect: fd: %d tm: %ld async: %d\n",
> -                     s, opt_tv ? tv.tv_sec : -1L, async);
> +     osip_debug(ld, "ldap_pvt_connect: fd: %d tm: %lld async: %d\n",
> +                     s, opt_tv ? (long long)tv.tv_sec : -1LL, async);
>  
>       if ( opt_tv && ldap_pvt_ndelay_on(ld, s) == -1 )
>               return ( -1 );
> --- a/libraries/libldap/os-local.c
> +++ b/libraries/libldap/os-local.c
> @@ -176,8 +176,8 @@ ldap_pvt_connect(LDAP *ld, ber_socket_t
>               opt_tv = &tv;
>       }
>  
> -     oslocal_debug(ld, "ldap_connect_timeout: fd: %d tm: %ld async: %d\n",
> -             s, opt_tv ? tv.tv_sec : -1L, async);
> +     oslocal_debug(ld, "ldap_connect_timeout: fd: %d tm: %lld async: %d\n",
> +             s, opt_tv ? (long long)tv.tv_sec : -1LL, async);
>  
>       if ( ldap_pvt_ndelay_on(ld, s) == -1 ) return -1;
>  
> --- a/libraries/libldap/result.c
> +++ b/libraries/libldap/result.c
> @@ -264,8 +264,8 @@ wait4msg(
>               Debug( LDAP_DEBUG_TRACE, "wait4msg ld %p msgid %d (infinite 
> timeout)\n",
>                       (void *)ld, msgid, 0 );
>       } else {
> -             Debug( LDAP_DEBUG_TRACE, "wait4msg ld %p msgid %d (timeout %ld 
> usec)\n",
> -                     (void *)ld, msgid, (long)timeout->tv_sec * 1000000 + 
> timeout->tv_usec );
> +             Debug( LDAP_DEBUG_TRACE, "wait4msg ld %p msgid %d (timeout %lld 
> usec)\n",
> +                     (void *)ld, msgid, (long long)timeout->tv_sec * 
> 1000000LL + timeout->tv_usec
> );
>       }
>  #endif /* LDAP_DEBUG */
>  
> --- a/servers/slapd/back-ldap/bind.c
> +++ b/servers/slapd/back-ldap/bind.c
> @@ -3008,14 +3008,14 @@ ldap_back_conn2str( const ldapconn_base_
>       }
>  
>       if ( lc->lcb_create_time != 0 ) {
> -             len = snprintf( tbuf, sizeof(tbuf), "%ld", lc->lcb_create_time 
> );
> +             len = snprintf( tbuf, sizeof(tbuf), "%lld", (long 
> long)lc->lcb_create_time
> );
>               if ( ptr + sizeof(" created=") + len >= end ) return -1;
>               ptr = lutil_strcopy( ptr, " created=" );
>               ptr = lutil_strcopy( ptr, tbuf );
>       }
>  
>       if ( lc->lcb_time != 0 ) {
> -             len = snprintf( tbuf, sizeof(tbuf), "%ld", lc->lcb_time );
> +             len = snprintf( tbuf, sizeof(tbuf), "%lld", (long 
> long)lc->lcb_time );
>               if ( ptr + sizeof(" modified=") + len >= end ) return -1;
>               ptr = lutil_strcopy( ptr, " modified=" );
>               ptr = lutil_strcopy( ptr, tbuf );
> --- a/servers/slapd/back-meta/config.c
> +++ b/servers/slapd/back-meta/config.c
> @@ -1277,8 +1277,8 @@ meta_back_cf_gen( ConfigArgs *c )
>                       if ( mc->mc_network_timeout == 0 ) {
>                               return 1;
>                       }
> -                     bv.bv_len = snprintf( c->cr_msg, sizeof(c->cr_msg), 
> "%ld",
> -                             mc->mc_network_timeout );
> +                     bv.bv_len = snprintf( c->cr_msg, sizeof(c->cr_msg), 
> "%lld",
> +                             (long long)mc->mc_network_timeout );
>                       bv.bv_val = c->cr_msg;
>                       value_add_one( &c->rvalue_vals, &bv );
>                       break;
> --- a/servers/slapd/overlays/dds.c
> +++ b/servers/slapd/overlays/dds.c
> @@ -417,7 +417,7 @@ dds_op_add( Operation *op, SlapReply *rs
>               assert( ttl <= DDS_RF2589_MAX_TTL );
>  
>               bv.bv_val = ttlbuf;
> -             bv.bv_len = snprintf( ttlbuf, sizeof( ttlbuf ), "%ld", ttl );
> +             bv.bv_len = snprintf( ttlbuf, sizeof( ttlbuf ), "%lld", (long 
> long)ttl );
>               assert( bv.bv_len < sizeof( ttlbuf ) );
>  
>               /* FIXME: apparently, values in op->ora_e are malloc'ed
> @@ -695,7 +695,7 @@ dds_op_modify( Operation *op, SlapReply
>                                       goto done;
>                               }
>  
> -                             bv_entryTtl.bv_len = snprintf( textbuf, sizeof( 
> textbuf ), "%ld", entryTtl
> );
> +                             bv_entryTtl.bv_len = snprintf( textbuf, sizeof( 
> textbuf ), "%lld", (long
> long)entryTtl );
>                               break;
>  
>                       default:
> @@ -917,7 +917,7 @@ dds_response( Operation *op, SlapReply *
>               ttl = (ttl < 0) ? 0 : ttl;
>               assert( ttl <= DDS_RF2589_MAX_TTL );
>  
> -             len = snprintf( ttlbuf, sizeof(ttlbuf), "%ld", ttl );
> +             len = snprintf( ttlbuf, sizeof(ttlbuf), "%lld", (long long)ttl 
> );
>               if ( len < 0 )
>               {
>                       goto done;
> @@ -1177,7 +1177,7 @@ dds_op_extended( Operation *op, SlapRepl
>               ttlmod.sml_values = ttlvalues;
>               ttlmod.sml_numvals = 1;
>               ttlvalues[ 0 ].bv_val = ttlbuf;
> -             ttlvalues[ 0 ].bv_len = snprintf( ttlbuf, sizeof( ttlbuf ), 
> "%ld", ttl );
> +             ttlvalues[ 0 ].bv_len = snprintf( ttlbuf, sizeof( ttlbuf ), 
> "%lld", (long
> long)ttl );
>               BER_BVZERO( &ttlvalues[ 1 ] );
>  
>               /* the entryExpireTimestamp is added by modify */
> @@ -1205,8 +1205,8 @@ dds_op_extended( Operation *op, SlapRepl
>                               rs->sr_rspoid = ch_strdup( 
> slap_EXOP_REFRESH.bv_val );
>  
>                               Log3( LDAP_DEBUG_TRACE, LDAP_LEVEL_INFO,
> -                                     "%s REFRESH dn=\"%s\" TTL=%ld\n",
> -                                     op->o_log_prefix, op->o_req_ndn.bv_val, 
> ttl );
> +                                     "%s REFRESH dn=\"%s\" TTL=%lld\n",
> +                                     op->o_log_prefix, op->o_req_ndn.bv_val, 
> (long long)ttl );
>                       }
>  
>                       ber_free_buf( ber );
> --- a/servers/slapd/overlays/pcache.c
> +++ b/servers/slapd/overlays/pcache.c
> @@ -2728,8 +2728,8 @@ pc_bind_search( Operation *op, SlapReply
>                                       pbi->bi_flags |= BI_HASHED;
>                       } else {
>                               Debug( pcache_debug, "pc_bind_search: cache is 
> stale, "
> -                                     "reftime: %ld, current time: %ld\n",
> -                                     pbi->bi_cq->bindref_time, op->o_time, 0 
> );
> +                                     "reftime: %lld, current time: %lld\n",
> +                                     (long long)pbi->bi_cq->bindref_time, 
> (long long)op->o_time, 0 );
>                       }
>               } else if ( pbi->bi_si ) {
>                       /* This search result is going into the cache */
> @@ -3831,9 +3831,9 @@ pc_cf_gen( ConfigArgs *c )
>               struct berval bv;
>               switch( c->type ) {
>               case PC_MAIN:
> -                     bv.bv_len = snprintf( c->cr_msg, sizeof( c->cr_msg ), 
> "%s %d %d %d %ld",
> +                     bv.bv_len = snprintf( c->cr_msg, sizeof( c->cr_msg ), 
> "%s %d %d %d %lld",
>                               cm->db.bd_info->bi_type, cm->max_entries, 
> cm->numattrsets,
> -                             cm->num_entries_limit, cm->cc_period );
> +                             cm->num_entries_limit, (long long)cm->cc_period 
> );
>                       bv.bv_val = c->cr_msg;
>                       value_add_one( &c->rvalue_vals, &bv );
>                       break;
> @@ -3875,12 +3875,12 @@ pc_cf_gen( ConfigArgs *c )
>                               /* HEADS-UP: always print all;
>                                * if optional == 0, ignore */
>                               bv.bv_len = snprintf( c->cr_msg, sizeof( 
> c->cr_msg ),
> -                                     " %d %ld %ld %ld %ld",
> +                                     " %d %lld %lld %lld %lld",
>                                       temp->attr_set_index,
> -                                     temp->ttl,
> -                                     temp->negttl,
> -                                     temp->limitttl,
> -                                     temp->ttr );
> +                                     (long long)temp->ttl,
> +                                     (long long)temp->negttl,
> +                                     (long long)temp->limitttl,
> +                                     (long long)temp->ttr );
>                               bv.bv_len += temp->querystr.bv_len + 2;
>                               bv.bv_val = ch_malloc( bv.bv_len+1 );
>                               ptr = bv.bv_val;
> @@ -3897,9 +3897,9 @@ pc_cf_gen( ConfigArgs *c )
>                       for (temp=qm->templates; temp; temp=temp->qmnext) {
>                               if ( !temp->bindttr ) continue;
>                               bv.bv_len = snprintf( c->cr_msg, sizeof( 
> c->cr_msg ),
> -                                     " %d %ld %s ",
> +                                     " %d %lld %s ",
>                                       temp->attr_set_index,
> -                                     temp->bindttr,
> +                                     (long long)temp->bindttr,
>                                       ldap_pvt_scope2str( temp->bindscope ));
>                               bv.bv_len += temp->bindbase.bv_len + 
> temp->bindftemp.bv_len + 4;
>                               bv.bv_val = ch_malloc( bv.bv_len + 1 );
> --- a/servers/slapd/syncrepl.c
> +++ b/servers/slapd/syncrepl.c
> @@ -2395,8 +2395,8 @@ syncrepl_message_to_op(
>       op->o_callback = &cb;
>       slap_op_time( &op->o_time, &op->o_tincr );
>  
> -     Debug( LDAP_DEBUG_SYNC, "syncrepl_message_to_op: %s tid %x\n",
> -             si->si_ridtxt, op->o_tid, 0 );
> +     Debug( LDAP_DEBUG_SYNC, "syncrepl_message_to_op: %s tid %lx\n",
> +             si->si_ridtxt, (unsigned long)op->o_tid, 0 );
>  
>       switch( op->o_tag ) {
>       case LDAP_REQ_ADD:
> @@ -2905,8 +2905,8 @@ syncrepl_entry(
>       int     freecsn = 1;
>  
>       Debug( LDAP_DEBUG_SYNC,
> -             "syncrepl_entry: %s LDAP_RES_SEARCH_ENTRY(LDAP_SYNC_%s) tid 
> %x\n",
> -             si->si_ridtxt, syncrepl_state2str( syncstate ), op->o_tid );
> +             "syncrepl_entry: %s LDAP_RES_SEARCH_ENTRY(LDAP_SYNC_%s) tid 
> %lx\n",
> +             si->si_ridtxt, syncrepl_state2str( syncstate ), (unsigned 
> long)op->o_tid );
>  
>       if (( syncstate == LDAP_SYNC_PRESENT || syncstate == LDAP_SYNC_ADD ) ) {
>               if ( !si->si_refreshPresent && !si->si_refreshDone ) {
> 
> 


-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/

Reply via email to