improved thread safety?
David Collier-Brown
davecb at sun.com
Tue Jan 22 14:28:01 GMT 2008
I've added a few more tested to my database, and found N more uses
of non-thread-safe function in Samba...
All of these are functions for which there are _r variants,
such as getgrent, for which there is a getgrent_r.
I've noted the ones that you seem to be handling already.
Name File Function Line
getnetgrent password.c validate_group 640 while (getnetgrent(&host, &user, &domain)
getspnam pass_check.c bool 710 spass = getspnam(pass->pw_name);
gmtime -- merely recommeded against, as the standards say
they're allowed to be thread-unsafe. I don't know
of any which aren't safe
rand -- used in a lot of test functions, and also in:
system.c sys_random 793 return (long )rand();
wb_common.c winbind_named_pipe_sock 261 slept = rand() % 3 + 1;
strtok -- only sane if used in exactly one thread
client.c cmd_mkdir 1481 p = strtok(ddir,"/\\");
client.c cmd_mkdir 1494 p = strtok(NULL,"/\\");
clitar.c ensurepath 544 p=strtok(ffname, "\\");
clitar.c ensurepath 561 p = strtok(NULL,"/\\");
smbmnt.c do_mount 175 major = strtok(release, ".");
smbmnt.c do_mount 176 minor = strtok(NULL, ".");
smbmount.c parse_mount_smb 816 for (opts = strtok(optarg,
afs_settoken.c afs_decode_token 61 if ((t = strtok(s, "\n")) ==
afs_settoken.c afs_decode_token 68 if ((t = strtok(NULL, "\n"))
afs_settoken.c afs_decode_token 78 if ((t = strtok(NULL, "\n"))
afs_settoken.c afs_decode_token 96 if ((t = strtok(NULL, "\n"))
afs_settoken.c afs_decode_token 106 if ((t = strtok(NULL, "\n"))
afs_settoken.c afs_decode_token 116 if ((t = strtok(NULL, "\n"))
afs_settoken.c afs_decode_token 126 if ((t = strtok(NULL, "\n"))
debug.c debug_parse_params 432 if ((class_name=strtok(params[i]
debug.c debug_parse_params 433 (class_level=strtok(NULL,
ads_struct.c ads_build_path 54 p=strtok(r,sep);
ads_struct.c ads_build_path 58 while ((p=strtok(NULL,sep)))
vfs_recycle.c recycle_create_dir 289 for (token = strtok(tok_str,
nmbd_processlogon.c process_logon_packet 447 while ((component = strtok(dc, "."))) {
loadparm.c lp_add_auto_services 4722 for (p = strtok(s, LIST_SEP);
load.c add_auto_printers 38 for (p = strtok(str,
lpq_parse.c parse_lpq_bsd 147 tok[0] = strtok(line2," \t");
lpq_parse.c parse_lpq_bsd 150 && ((tok[count] = strtok(NULL,"
print_aix.c aix_cache_reload 63 p = strtok(line, ":");
cmd_spoolss.c get_driver_3_param 1227 ptr = strtok(str, delim);
cmd_spoolss.c init_drv_info_3_members 1269 str = strtok(str, ",");
cmd_spoolss.c init_drv_info_3_members 1275 str = strtok(NULL, ",");
msdfs.c parse_msdfs_symlink 296 prot = strtok(temp,":");
msdfs.c parse_msdfs_symlink 309 strtok(NULL,",")) != NULL)) {
password.c authorise_login 771 for(auser=strtok(user_list,LIST_ SEP)
password.c authorise_login 772 auser =strtok(NULL,LIST_SEP)) {
password.c authorise_login 809 for(auser=strtok(user_list,LIST_
password.c authorise_login 810 auser =strtok(NULL,LIST_SEP)) {
cgi.c cgi_load_variables 167 for(tok=strtok(s,"&;");tok;
Already handled:
getgrent
getgrgid
getgrnam
getpwent
getpwnam
getpwuid
readdir
etc, etc...
Jelmer Vernooij wrote:
> Am Samstag, den 19.01.2008, 11:43 -0500 schrieb David Collier-Brown:
>
>>At the expense of writing you a long answer rather than a short one,
>>samba 4 uses the following functions which are mt-unsafe on Solaris
>>and other standard-compliant systems:
>
> Thanks, this is very useful.
>
>
>> gethostbyaddr
>> gethostbyname
>
> I've filed bugs about these two.
>
>
>> getopt
>
> The calls to getopt() are not in code that uses threads or is used in
> libraries.
>
>
>> getpass
>
> We do use this in library code (credentials code), but I'm not quite
> sure how to fix this without using locks. There's only one prompt you
> can display at the same time, anyway.
>
>
>> getservbyport
>
> This is only used in the replacement code.
>
>
>> inet_addr
>
> I don't see why this one would be thread-unsafe.
Aha: I checked the man page, and the function is mt-save. I've
fixed the database for it and initgroups, below...
Thanks, I've now complete re-checked the database and found
several more functions where the MT-level had been corrected since
Solaris 8. Ecvt, fcvt and popen, for example!
>
>
>> inet_ntoa
>
> I've filed a bug about this one.
>
>
>> pclose
>> popen
>
> Using grep, I couldn't find any references to popen/pclose.
fixed
>
>
>> initgroups
>
> I'm not sure why this would be thread unsafe.
fixed
>
>
>> random
>> srandom
>
> I'm not sure what to do about these. Are there any better alternatives?
>
>
>> system
>
> This isn't used by any code that uses threads or any library code except
> the fault handling.
>
>
>>I have a tool which finds these, and it found 290 lines of them.
>>such as:
>>
>>"GIT/v4-0-test/source/lib/replace/getaddrinfo.c", line 110: gethostbyaddr
>># gethostbyaddr
>># gethostbyaddr - network host database functions
>># MT-Level: MT-Unsafe
>
> The code in lib/replace/ is meant to provide replacements for functions
> on platforms that don't have them and may use thread-unsafe functions to
> implement them. Other parts of Samba should not use thread-unsafe
> functions but instead rely on the thread-safe functions provided by the
> system or lib/replace.
>
>
>>The non-comment lines are in error-message format, to make it
>>easy to use vim or emacs to review them all, and see if the
>>code around them uses locks or other techniques to make them
>>survive multi-threading.
>>
>>Send me mail if you want data on mt-unsafe, non-posix, or
>>32-bit-only only code: the tool is really for porting, but
>>can inspect for all sorts of other things.
>
> Yeah, I would definitely be interested in that. Is this tool freely
> available somewhere? It would be interesting to have it running as part
> of the build process, testsuite or the buildfarm.
>
> Cheers,
>
> Jelmer
> S
--
David Collier-Brown | Always do right. This will gratify
Sun Microsystems, Toronto | some people and astonish the rest
davecb at sun.com | -- Mark Twain
(800) 555-9786 x56583 cell: (647) 833-9377 home off: (416) 223-5943
More information about the samba-technical
mailing list