Bug 1243121 Comment 57 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by

ISHIKAWA, Chiaki

on 2021-03-12 00:41:20 PST

After the previous patch was applied, the build proceeds with warnings below.
Because the previous patch touches a header file, some files were re-compiled, a some more warnings were printed (unused variables, etc.)

Now, after a few years of thinking of about the patches and a fresh look at the source code, though, 
I decided to change |ber_tag_t|  to |int|.
 
This is after I thought hard about the signed vs unsigned extension issue.
- A few functions return |ber_tag_t| as the next tag read from input stream.
- A few functions return the length of a string or something to that effect.
  However, upon error, these functions return 0xFFFFFFFF to signal an error.
  Defining  |ber_tag_t| to be |int| causes a tricky issue of sign extension for these length returning functions, but still I felt it would be more advantageous for code cleanup reasons to do so.
Thus I defined this AFTER all these years having created the modifications without doing so.

Anyway here is the patch.


```
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:86:37: warning: the address of ‘buf’ will always evaluate as ‘true’ [-Waddress]
   86 | #define NSLDAPI_SAFE_STRLEN(s) ((s) ? strlen((s)) + 1 : 1)
      |                                     ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:591:9: note: in expansion of macro ‘NSLDAPI_SAFE_STRLEN’
  591 |   len = NSLDAPI_SAFE_STRLEN(buf) + NSLDAPI_SAFE_STRLEN(tmpbase) +
      |         ^~~~~~~~~~~~~~~~~~~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:85:37: warning: the address of ‘buf’ will always evaluate as ‘true’ [-Waddress]
   85 | #define NSLDAPI_STR_NONNULL(s) ((s) ? (s) : "")
      |                                     ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:623:11: note: in expansion of macro ‘NSLDAPI_STR_NONNULL’
  623 |           NSLDAPI_STR_NONNULL(buf));
      |           ^~~~~~~~~~~~~~~~~~~
In file included from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:45:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘memcache_adj_size’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:912:36: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
  912 |     assert(cache->ldmemc_size_used >= 0);
      |                                    ^~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:912:36: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
  912 |     assert(cache->ldmemc_size_used >= 0);
      |                                    ^~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘memcache_access’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1451:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 1451 |     int scope = (int)pData2;
      |                 ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘memcache_flush’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1557:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 1557 |                       (void*)scope, NULL);
      |                       ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1559:64: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 1559 |       memcache_access(cache, MEMCACHE_ACCESS_FLUSH, (void*)dn, (void*)scope,
      |                                                                ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘msgid_hashf’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1717:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 1717 |   unsigned code = (unsigned)((ldapmemcacheReqId*)key)->ldmemcrid_ld;
      |                   ^

      ...


/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/os-ip.c: In function ‘nsldapi_os_connect_with_to’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/os-ip.c:360:65: warning: pointer targets in passing argument 5 of ‘getsockopt’ differ in signedness [-Wpointer-sign]
  360 |     if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) < 0)
      |                                                                 ^~~~
      |                                                                 |
      |                                                                 int *
In file included from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/ldap-int.h:61,
                 from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/os-ip.c:67:
/usr/include/x86_64-linux-gnu/sys/socket.h:210:32: note: expected ‘socklen_t * restrict’ {aka ‘unsigned int * restrict’} but argument is of type ‘int *’
  210 |          socklen_t *__restrict __optlen) __THROW;
      |          ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~


      ...

 -MP -MF .deps/tmplout.o.pp  -fdiagnostics-color  /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c
comm/ldap/c-sdk/libraries/libldap/ufn.o
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c: In function ‘do_entry2text_search’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c:395:20: warning: variable ‘html’ set but not used [-Wunused-but-set-variable]
  395 |   int err, freedn, html;
      |                    ^~~~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c: In function ‘searchaction’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c:920:38: warning: variable ‘selectname’ set but not used [-Wunused-but-set-variable]
  920 |   char *value, *filtpattern, *attr, *selectname;
      |                                      ^~~~~~~~~~


```

Revision 1 by

ISHIKAWA, Chiaki

on 2021-03-12 02:46:16 PST

After the previous patch was applied, the build proceeds with warnings below.
Because the previous patch touches a header file, some files were re-compiled, amd some more warnings were printed (unused variables, etc.)

Now, after a few years of thinking of about the patches and a fresh look at the source code, though, 
I decided to change |ber_tag_t|  to |int|.
 
This is after I thought hard about the signed vs unsigned extension issue.
- A few functions return |ber_tag_t| as the next tag read from input stream.
- A few functions return the length of a string or something to that effect.
  However, upon error, these functions return 0xFFFFFFFF to signal an error.
  Defining  |ber_tag_t| to be |int| causes a tricky issue of sign extension for these length returning functions, but still I felt it would be more advantageous for code cleanup reasons to do so.
Thus I defined this AFTER all these years having created the modifications without doing so.

Anyway here is the patch.


```
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:86:37: warning: the address of ‘buf’ will always evaluate as ‘true’ [-Waddress]
   86 | #define NSLDAPI_SAFE_STRLEN(s) ((s) ? strlen((s)) + 1 : 1)
      |                                     ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:591:9: note: in expansion of macro ‘NSLDAPI_SAFE_STRLEN’
  591 |   len = NSLDAPI_SAFE_STRLEN(buf) + NSLDAPI_SAFE_STRLEN(tmpbase) +
      |         ^~~~~~~~~~~~~~~~~~~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:85:37: warning: the address of ‘buf’ will always evaluate as ‘true’ [-Waddress]
   85 | #define NSLDAPI_STR_NONNULL(s) ((s) ? (s) : "")
      |                                     ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:623:11: note: in expansion of macro ‘NSLDAPI_STR_NONNULL’
  623 |           NSLDAPI_STR_NONNULL(buf));
      |           ^~~~~~~~~~~~~~~~~~~
In file included from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:45:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘memcache_adj_size’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:912:36: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
  912 |     assert(cache->ldmemc_size_used >= 0);
      |                                    ^~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:912:36: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
  912 |     assert(cache->ldmemc_size_used >= 0);
      |                                    ^~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘memcache_access’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1451:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 1451 |     int scope = (int)pData2;
      |                 ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘memcache_flush’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1557:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 1557 |                       (void*)scope, NULL);
      |                       ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1559:64: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 1559 |       memcache_access(cache, MEMCACHE_ACCESS_FLUSH, (void*)dn, (void*)scope,
      |                                                                ^
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c: In function ‘msgid_hashf’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/memcache.c:1717:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 1717 |   unsigned code = (unsigned)((ldapmemcacheReqId*)key)->ldmemcrid_ld;
      |                   ^

      ...


/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/os-ip.c: In function ‘nsldapi_os_connect_with_to’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/os-ip.c:360:65: warning: pointer targets in passing argument 5 of ‘getsockopt’ differ in signedness [-Wpointer-sign]
  360 |     if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) < 0)
      |                                                                 ^~~~
      |                                                                 |
      |                                                                 int *
In file included from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/ldap-int.h:61,
                 from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/os-ip.c:67:
/usr/include/x86_64-linux-gnu/sys/socket.h:210:32: note: expected ‘socklen_t * restrict’ {aka ‘unsigned int * restrict’} but argument is of type ‘int *’
  210 |          socklen_t *__restrict __optlen) __THROW;
      |          ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~


      ...

 -MP -MF .deps/tmplout.o.pp  -fdiagnostics-color  /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c
comm/ldap/c-sdk/libraries/libldap/ufn.o
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c: In function ‘do_entry2text_search’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c:395:20: warning: variable ‘html’ set but not used [-Wunused-but-set-variable]
  395 |   int err, freedn, html;
      |                    ^~~~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c: In function ‘searchaction’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/ldap/c-sdk/libraries/libldap/tmplout.c:920:38: warning: variable ‘selectname’ set but not used [-Wunused-but-set-variable]
  920 |   char *value, *filtpattern, *attr, *selectname;
      |                                      ^~~~~~~~~~


```

Back to Bug 1243121 Comment 57

Bugzilla

Quick Search

Bug 1243121 Comment 57 Edit History