Samba-2.2.8pre2 compiler warnings
Richard Sharpe
rsharpe at richardsharpe.com
Mon Mar 3 02:10:15 GMT 2003
On Sun, 2 Mar 2003, Brian Poole wrote:
> Quoting Nicholas Brealey (nick at brealey.org) from 1 March 2003:
> >
> > CLITAR
> > ======
> >
> > "client/clitar.c", line 688: warning: argument #4 is incompatible with
> > prototype:
> > prototype: pointer to uint : "include/proto.h", line 303
> > argument : pointer to ullong
> >
> > A 64 bit integer being used where a 32 bit integer is expected?
> > smbtar make not work on bigendian machines?
> > smbtar may not work for files over 2GB on little endian machines?
Groan. I need to look at the CIFS def for the size of that field and how
it is handled if large offsets are in use :-(
> Hmm.. I don't know what to say here. cli_getattrE does expect an int..
> but if its being provided with the filesize an int isn't going to
> suffice..
>
> smbtar still has at least two issues that could cause problems
> with large files to my knowledge. I'll repost the list to keep it alive
> and in developers' minds. This has been given to rsharpe & herb
> previously. I wouldn't be terribly suprised if there are more large file
> problems hiding in the tar code as it doesn't seem like it gets much use
> and/or attention from the developers.
>
> 1) clitar.c::get_file()
>
> static int get_file(file_info2 finfo)
> {
> int fnum = -1, pos = 0, dsize = 0, rsize = 0, bpos = 0;
> <..>
> rsize = finfo.size; /* This is how much to write */
>
> rsize would overflow here.
>
> 2) clitar.c::get_longfilename()
>
> static char * get_longfilename(file_info2 finfo)
>
> int namesize = finfo.size + strlen(cur_dir) + 2;
> char *longname = malloc(namesize);
>
> This must not be frequently used code. This would [attempt] to
> malloc a 1GB chunk of memory (if that was the size of the current file.)
> This should be using strlen(finfo.name) or something.
>
> At least a couple debugs still bogus in clitar.c 2_2 code as well.. I
> believe one of these got fixed in HEAD but not 2_2.
>
> DEBUG(5, ("get_file: file: %s, size %i\n", finfo.name, (int)finfo.size));
>
> (can't cast finfo.size to a int)
>
> DEBUG(0, ("restore tar file %s of size %d bytes\n", finfo.name, (int)finfo.size));
>
> (can't cast finfo.size to a int)
> I've attached a patch for the issues I know of (but not the cli_getattrE
> issue that you brought up.) I'm not really sure what to say about that.
> The patch is against the 2_2 tree but should mostly apply to HEAD as well.
> This patch was tested only for compilation and should be treated as such.
>
>
> -b
>
> Index: clitar.c
> ===================================================================
> RCS file: /cvsroot/samba/source/client/clitar.c,v
> retrieving revision 1.74.4.10
> diff -u -r1.74.4.10 clitar.c
> --- clitar.c 6 Jan 2003 19:53:11 -0000 1.74.4.10
> +++ clitar.c 2 Mar 2003 23:41:47 -0000
> @@ -1000,9 +1000,10 @@
>
> static int get_file(file_info2 finfo)
> {
> - int fnum = -1, pos = 0, dsize = 0, rsize = 0, bpos = 0;
> + int fnum = -1, pos = 0, dsize = 0, bpos = 0;
> + SMB_BIG_INT rsize = 0;
Got that one ...
> - DEBUG(5, ("get_file: file: %s, size %i\n", finfo.name, (int)finfo.size));
> + DEBUG(5, ("get_file: file: %s, size %.0f\n", finfo.name, (double)finfo.size));
Hmmm, this should be llu if sizeof(off_t) is 8, and lu if sizeof(off_t) is
4, but maybe casting to a float is OK.
> if (ensurepath(finfo.name) &&
> (fnum=cli_open(cli, finfo.name, O_RDWR|O_CREAT|O_TRUNC, DENY_NONE)) == -1) {
> @@ -1093,7 +1094,7 @@
>
> ntarf++;
>
> - DEBUG(0, ("restore tar file %s of size %d bytes\n", finfo.name, (int)finfo.size));
> + DEBUG(0, ("restore tar file %s of size %.0f bytes\n", finfo.name, (double)finfo.size));
>
> return(True);
> }
> @@ -1123,18 +1124,17 @@
> */
> static char * get_longfilename(file_info2 finfo)
> {
> - int namesize = finfo.size + strlen(cur_dir) + 2;
> + int namesize = strlen(finfo.name) + strlen(cur_dir) + 2;
> char *longname = malloc(namesize);
> - int offset = 0, left = finfo.size;
> + SMB_BIG_INT offset = 0, left = finfo.size;
> BOOL first = True;
>
> DEBUG(5, ("Restoring a long file name: %s\n", finfo.name));
> - DEBUG(5, ("Len = %d\n", (int)finfo.size));
> + DEBUG(5, ("Len = %.0f\n", (double)finfo.size));
>
> if (longname == NULL) {
> -
> DEBUG(0, ("could not allocate buffer of size %d for longname\n",
> - (int)(finfo.size + strlen(cur_dir) + 2)));
> + namesize));
> return(NULL);
> }
>
OK, I got all those, thanks ...
Regards
-----
Richard Sharpe, rsharpe[at]ns.aus.com, rsharpe[at]samba.org,
sharpe[at]ethereal.com, http://www.richardsharpe.com
More information about the samba-technical
mailing list