- Unused variables in upnpdebug.c in release builds
- Cast to invalid integer size in BSD systems, pointers should be cast to unsigned long
instead of unsigned int.
Please send two different patches for two different fixes, it saves me a lot of work.
I will commit the debug part, but I am not sure about the ThreadPool.c one. Why is it an invalid integer size? Do you mean srand() has an unsigned long argument in BSD? Even then, I see no problem in that code, since all we want is to generate a random seed.
Regards,
Marcelo.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
In the future, please let me know when you feel a patch addresses different issues, and I'll be happy to reorganize the patch. In this particular case, I felt the patch was a fix for single issue of compiler warnings, but I can also see how these are separate fixes. My goal is definitely to make things as easy as possible on your end.
Re: "Do you mean srand() has an unsigned long argument in BSD?"
No, pointers in BSD seem to be unsigned long, which is currently being cast as a (unsigned int), which is generating the warning. You're right. Functionally, there's no problem with the code. It's a trivial compilation warning.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Ok, I'll let you know from now on. I usually say that because most people don't feel happy to reorganize their patch, so I keep hoping for the next time :)
Well, this is something peculiar. I don't understand why your compiler is generating a warning. What matters in a cast is where the value is going to be used, so if you convert it to something else, there should be no problem. But the compiler could be acknowledging loss of precision or something like this. Even then, I have never seen a warning because someone did int y = -1; unsigned char x = (unsigned char)y; The purpose of casts would be to shut up the compiler.
My system is 64 bits, and longs here are a different size from ints, and even then I get no warning. And then there would be a reason for the compiler to complain, sing srand() takes an unsigned int argument, not an unsigned long.
Could you please post the warning? Do you think that two casts in a row would make the compiler happy? Something like (unsigned int)(unsigned long)?
Regards,
Marcelo.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
A cast in principle tells the compiler to shut up. And yet, gcc feels in the right to speak even though you casted the result to a different size. You can't be more explicit about your wish than casting. But gcc does not complain that after you cast it to unsigned long you will be passing an integer of a bigger size (unsigned long) to a function that accepts a smaller size (unsinged int), at least in 64 bits, and in this case you loose precision.
That is weird behaviour, IMHO.
What is your suggestion in this case? Would "(unsigned int)(unsigned long)p" do the trick?
Regards,
Marcelo.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Fix for issue
Hi Chandra,
Please send two different patches for two different fixes, it saves me a lot of work.
I will commit the debug part, but I am not sure about the ThreadPool.c one. Why is it an invalid integer size? Do you mean srand() has an unsigned long argument in BSD? Even then, I see no problem in that code, since all we want is to generate a random seed.
Regards,
Marcelo.
Hi Marcelo,
In the future, please let me know when you feel a patch addresses different issues, and I'll be happy to reorganize the patch. In this particular case, I felt the patch was a fix for single issue of compiler warnings, but I can also see how these are separate fixes. My goal is definitely to make things as easy as possible on your end.
Re: "Do you mean srand() has an unsigned long argument in BSD?"
No, pointers in BSD seem to be unsigned long, which is currently being cast as a (unsigned int), which is generating the warning. You're right. Functionally, there's no problem with the code. It's a trivial compilation warning.
Hi Chandra,
Ok, I'll let you know from now on. I usually say that because most people don't feel happy to reorganize their patch, so I keep hoping for the next time :)
Well, this is something peculiar. I don't understand why your compiler is generating a warning. What matters in a cast is where the value is going to be used, so if you convert it to something else, there should be no problem. But the compiler could be acknowledging loss of precision or something like this. Even then, I have never seen a warning because someone did int y = -1; unsigned char x = (unsigned char)y; The purpose of casts would be to shut up the compiler.
My system is 64 bits, and longs here are a different size from ints, and even then I get no warning. And then there would be a reason for the compiler to complain, sing srand() takes an unsigned int argument, not an unsigned long.
Could you please post the warning? Do you think that two casts in a row would make the compiler happy? Something like (unsigned int)(unsigned long)?
Regards,
Marcelo.
Hi Marcelo,
Yea, I definitely don't mind, especially since there are a lot of patches coming in.
Anyways, here's the compiler warning:
src/ThreadPool.c: In function 'SetSeed':
src/ThreadPool.c:419: warning: cast from pointer to integer of different size
Here's the output of gcc -v:
Using built-in specs.
Target: i686-apple-darwin10
Configured with: /var/tmp/gcc/gcc-5664~38/src/configure --disable-checking --enable-werror --prefix=/usr --mandir=/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.2/ --with-slibdir=/usr/lib --build=i686-apple-darwin10 --program-prefix=i686-apple-darwin10- --host=x86_64-apple-darwin10 --target=i686-apple-darwin10 --with-gxx-include-dir=/include/c++/4.2.1
Thread model: posix
gcc version 4.2.1 (Apple Inc. build 5664)
Thanks,
Chandra
Hi Chandra,
A cast in principle tells the compiler to shut up. And yet, gcc feels in the right to speak even though you casted the result to a different size. You can't be more explicit about your wish than casting. But gcc does not complain that after you cast it to unsigned long you will be passing an integer of a bigger size (unsigned long) to a function that accepts a smaller size (unsinged int), at least in 64 bits, and in this case you loose precision.
That is weird behaviour, IMHO.
What is your suggestion in this case? Would "(unsigned int)(unsigned long)p" do the trick?
Regards,
Marcelo.