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? */ } --- 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 ); --- 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 ); --- 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 ) {
tg@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. Thanks. Most of these look OK, but the unconditional use of "long long" instead of "long" will break on machines where "long long" is not 64 bits. > > 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? */ > } > --- 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 ); > --- 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 ); > --- 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/
Hi, > Thanks. Most of these look OK, but the unconditional use of "long > long" instead of "long" will break on machines where "long long" is > not 64 bits. 64 bits is not the requirement for all those, but size of time_t. Each of these instances was changed from outputting a time_t with %ld (without cast to long) to outputting a time_t with %lld after being cast to long long. As long long is either the same size or greater than long, this cannot break any existing scenario. It merely fixes the scenario in which time_t is larger than long but fits into long long. (It also fixes the case where time_t is smaller than long, incidentally, as it was not cast, prior.) Actual bit sizes are of no import here due to the explicit casts, given that time_t can be of various sizes and, in constrast to e.g. size_t with its %zu specifier, does not have a printf specifier of its own. Fixing this is already common practice for me, as MirBSD/i386 is also a platform with 32-bit long and 64-bit time_t, and has been for 14 years and about five weeks or so. bye, //mirabilos -- When he found out that the m68k port was in a pretty bad shape, he did not, like many before him, shrug and move on; instead, he took it upon himself to start compiling things, just so he could compile his shell. How's that for dedication. -- Wouter, about my Debian/m68k revival
tg@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/
Howard Chu dixit: >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. There are multiple systems on which sizeof(time_t) > sizeof(long), with more expected as 2038 dawns. Are you saying you want to prohibit users on these platforms from debugging OpenLDAP? >> --- 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] ); This doesn’t look like debugging output to me. bye, //mirabilos -- Yes, I hate users and I want them to suffer. -- Marco d'Itri on gmane.linux.debian.devel.general
Thorsten Glaser wrote: > Howard Chu dixit: > >> 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. > > There are multiple systems on which sizeof(time_t) > sizeof(long), > with more expected as 2038 dawns. > > Are you saying you want to prohibit users on these platforms from > debugging OpenLDAP? > >>> --- 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] ); > > This doesn’t look like debugging output to me. I already addressed that patch explicitly. "The rest" covers all the others. You might have correctly identified a Y2038 issue. You certainly have not written a correct fix for it. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
Howard Chu dixit: >You might have correctly identified a Y2038 issue. You certainly have not >written a correct fix for it. Please drop this condescending attitude. I may or may not have discovered a Y2038 issue, but I do know for sure I’ve correctly fixed a problem on all architectures in which a time_t does not fit into a long. Consider this: #ifdef TEST1 typedef long foo_t; #else typedef long long foo_t; #endif foo_t bla = 1L; printf("%ld, %ld\n", bla, 2L); This is _supposed_ to display “1, 2” but will display “1, 0” on little endian platforms (big endian being worse off). I know my way *very* well around the time_t issue because I’ve been having them on MirBSD/i386 years before x32 even existed, and am experienced in how to fix them. bye, //mirabilos -- 16:47⎜«mika:#grml» .oO(mira ist einfach gut....) 23:22⎜«mikap:#grml» mirabilos: und dein bootloader ist geil :) 23:29⎜«mikap:#grml» und ich finds saugeil dass ich ein bsd zum booten mit grml hab, das muss ich dann gleich mal auf usb-stick installieren -- Michael Prokop über MirOS bsd4grml
Thorsten Glaser wrote: > I know my way *very* well around the time_t issue because > I’ve been having them on MirBSD/i386 years before x32 even > existed, and am experienced in how to fix them. From here it only looks like you're experienced in how to break things. This patch of yours *introduces a bug*. If you wanted to actually *fix* something you should have made sure the malloc'd buffer - which only happens 2 lines above the lines you change - was large enough to store a long long. That's just plain sloppy. It's as careless as submitting a patch to fix a no-op, and just wasting my time. +--- 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; -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
Howard Chu dixit: >This patch of yours *introduces a bug*. If you wanted to actually *fix* >something you should have made sure the malloc'd buffer - which only happens >2 lines above the lines you change - was large enough to store a long long. That’s not a new bug, then. That’s a completely separate Y2038 issue of *yours* that will happen on all platforms, and has nothing to do with long vs. long long, since a long on an LP64 platform *is* already as wide as a long long. In addition to that, use of snprintf will prevent this from being worse than a mere truncation. bye, //mirabilos -- Solange man keine schmutzigen Tricks macht, und ich meine *wirklich* schmutzige Tricks, wie bei einer doppelt verketteten Liste beide Pointer XORen und in nur einem Word speichern, funktioniert Boehm ganz hervorragend. -- Andreas Bogk über boehm-gc in d.a.s.r
Thorsten Glaser wrote: > Howard Chu dixit: > >> This patch of yours *introduces a bug*. If you wanted to actually *fix* >> something you should have made sure the malloc'd buffer - which only happens >> 2 lines above the lines you change - was large enough to store a long long. > > That’s not a new bug, then. > That’s a completely separate Y2038 issue of *yours* that > will happen on all platforms, and has nothing to do with > long vs. long long, since a long on an LP64 platform *is* > already as wide as a long long. Thanks to the wonderful geniuses at Microsoft, not all the world uses LP64. IL32 is a thing. And this code works on those platforms. Far more machines than have ever or ever will run X32. If you want patches for your obscure system adopted, you have to make sure not to break anything else. > In addition to that, use of snprintf will prevent this > from being worse than a mere truncation. If you're going to submit a patch, submit a correct one. It's that simple. If you refuse to do that, then go away. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
Howard Chu dixit: >Thanks to the wonderful geniuses at Microsoft, not all the world >uses LP64. IL32 is a thing. And this code works on those platforms. These changes do not break LLP64 (this is the correct name for the 64-bit Windows model). >If you're going to submit a patch, submit a correct one. It's that The buffer size is a separate issue from this. Feel free to fix it, now that you found it. The patch I submit fixed your software on systems where time_t doesn’t fit into a long, all of them, and that *will* include 64-bit Windows at least in 2038, except some architectures need these fixes right now. They don’t break anything else before 2038, and afterwards, nothing new is broken worsely either. >simple. If you refuse to do that, then go away. You know what, I’ll just do that and leave you to fix your software on your own, clearly you are oh so utterly competent in doing that. //mirabilos -- “It is inappropriate to require that a time represented as seconds since the Epoch precisely represent the number of seconds between the referenced time and the Epoch.” -- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2
Thorsten Glaser wrote: > Howard Chu dixit: > >> Thanks to the wonderful geniuses at Microsoft, not all the world >> uses LP64. IL32 is a thing. And this code works on those platforms. > > These changes do not break LLP64 (this is the correct name > for the 64-bit Windows model). Yes, they do, but you're clearly too stupid to understand that, despite having had it explained to you multiple times. Your patch tries to store the value of a long long into a buffer only large enough for a long. That is fucking stupid. Bye. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
Y2038 issue needs resolving
Need to examine any use of timet and what platforms may have a 32-bit long.
The bug is mistitled. It should be “adapt to 64-bit time_t on systems with 32-bit long”, which has become a fairly standard setup in the last 15 years and will become even more so in the next… less than 17 years, I suppose.
FWIW, Debian is going to switch 32-bit ARM (with 32-bit long) to 64-bit time_t and off_t soon, and others, even m68k, will follow.
Debian and Ubuntu have both switched their remaining 32-bit architectures, except for i686, to 64-bit time_t. The change is in Ubuntu 24.04 (already released) and Debian 13/trixie (not yet released). Steve Langasek committed this distro patch: https://salsa.debian.org/openldap-team/openldap/-/blob/2a8f9240b9b6fd577d91352d6de858fadf586636/debian/patches/64-bit-time-t-compat.patch It's mostly the same as what was previously proposed in this ITS (changing %ld format specifiers to %lld), and unfortunately contains the same smbk5pwd bug that was already commented on. I didn't understand Howard's comment ('the unconditional use of "long long" instead of "long" will break on machines where "long long" is not 64 bits'). My understanding is C specifies "long long" to be at least 64 bits, and I'm not aware of any existing systems (yet) where "long long" is 128 bits - is it more of a futureproofing concern? Casting to long long and formatting with %lld seems to be the generally accepted solution in the broader community. If that's not acceptable, maybe scripting configure to generate a PRI_TIME_T format specifier? Steve's patch comment mentions an assertion failure in test046-dds on 32-bit ARM: servers/slapd/overlays/dds.c:422: dds_op_add: Assertion `bv.bv_len < sizeof( ttlbuf )' failed. I have not reproduced it myself (I don't have ARM hardware, and it isn't happening for me on x86). I note that the assertion ttl <= DDS_RF2589_MAX_TTL just above did not fail; but that does not rule out corruption of either the 64-bit value (could be negative) or the 32-bit quantity read by snprintf. I haven't figured out what actually happened here, but it's irritating me.
On Sat, 21 Sep 2024, openldap-its@openldap.org wrote: >https://bugs.openldap.org/show_bug.cgi?id=8890 >I didn't understand Howard's comment ('the unconditional use of "long long" […] Looking at it *again*, with some years of distance, I *think* I now see what Howard’s barely comprehensible and rather rude comments were meant to point out. I did not see it at that time because ⓐ I was trying, and, as a nōn-English-native speaker, failing to puzzle out what he was saying, and ⓑ the OpenLDAP code’s use of abstractions here has massively reduced its legibility. + 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()); I think the first line of that is the point of critique. On repeat reading, it looks like it tries to figure out how many bytes are needed to represent a long, then allocates a buffer sized by this. As I correctly pointed out, this is a separate issue. However, Howard rejected both the immediate fix of printing wrong data RIGHT NOW (and likely truncating that at some point in the future) and this follow-up bug to address that truncation. AIUI, this should likely be something like: +- keys[0].bv_val = ch_malloc( LDAP_PVT_INTTYPE_CHARS(long) ); ++ keys[0].bv_val = ch_malloc( LDAP_PVT_INTTYPE_CHARS(long 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()); Of course, someone who actually knows wth LDAP_PVT_INTTYPE_CHARS is needs to ensure that it DTRT for an argument of “long long” first. bye, //mirabilos
On Sat, 21 Sep 2024, tg@debian.org wrote: >AIUI, this should likely be something like: … make that… +- keys[0].bv_val = ch_malloc( LDAP_PVT_INTTYPE_CHARS(long) ); ++ keys[0].bv_val = ch_malloc( LDAP_PVT_INTTYPE_CHARS(long long) ); + keys[0].bv_len = snprintf(keys[0].bv_val, +- LDAP_PVT_INTTYPE_CHARS(long), ++ LDAP_PVT_INTTYPE_CHARS(long long), +- "%ld", slap_get_time()); ++ "%lld", (long long)slap_get_time()); … of course. (It’s 03:04 in the night, and re-reading those comments from above was not effortless.) bye, //mirabilos