On 2009/01/12 18:25, Gunnar Roth <gunnar.r...@gmx.de> wrote:
> Well "--enable-werror  maybe it should be default.
> Problem on windows vs 2008  is that i get a lot other warnings, so cannot
> make warnings to errors.

No, it will not be default.  When a user compiles MPD, it should not
fail just because of a warning.  New warnings are introduced by new
gcc versions quite often, and untested library versions may be bugged,
so we don't have full control over the warnings which users may
experience.

> Well before i did only change xClose but then it was removed so i changed 

Changing xclose() was plain wrong!  This breakss closing regular file
descriptors.

>> Also: don't code in CPP when you can code in C.  There are many
>> advantages in inline functions compared to macros.
>
> Of course you are right.
> So i made a inline function
> inline closesocket(int fd)
> {
> close(fd);
> }
> in utils.h ( oscompat.h was removed from git)
>
> added g_io_channel_new_socket and g_io_channel_new_fd inline func in utils.h

I dislike the whole utils.h, but it's ok for new.

>>>> Care to explain this one?  On Unix, it's enough to listen for G_IO_IN,
>>>> since the socket becomes readable when the peer closes the connection.
>>>
>>> I dont know what happen on unix.
>>> on windows the connect list run full ,when i was in the debugger  and
>>> stepped.
>>
>> Trial'n'error is the wrong way to develop software.  Do some research
>> instead of trying until it succeeds.  This leads to fragile code.
>>
>
> i was debbuging some other stuuf ( better see how the code flows
> ) but due to debugger stops mpd did not answer to the client, which 
> opened more conections then.

So your problem was unrelated to G_IO_NVAL|G_IO_HUP|G_IO_ERR missing?

>> This is executed when read() returns a non-positive value (except for
>> EINTR).  This is known to work well on POSIX compliant systems.
>
> So it does not work with glib on windows it seems.

Let's not talk about what "seems" to work, but what the documentation
says.  I'm not a windows expert, so it's up to you to read it.

> hmm ok, wxwidgets uses this:
> // return the directory with system config files:
>    // /etc under Unix, c:\Documents and Settings\All Users\Application Data
>    // under Windows, /Library/Preferences for Mac
>    virtual wxString GetConfigDir() const = 0;
>
> but to get this,  its not just reading a env var.
> ( need to call  SHGetSpecialFolderLocation with CSIDL_COMMON_APPDATA, 
> feéd item list result to SHGetPathFromIDListto get actual string, call  
> SHGetMalloc to free the item list)

I asked you what the Microsoft documentation says about that, and I
don't care much what wxWindows is doing.  It might be wrong.

>>> maybe it should be configurable, if the files must belong to the mpd  
>>> user.
>>> as this is a rather hard restrinction, because no files of me would
>>> belong to this user.
>>
>> Why not use the same semantics as MPD on Unix?
>
> I meant unix. ( i use mpd on linux actually)

You didn't describe MPD's semantics on Unix at all.  MPD does not
require ownership on music files.

> For windows i thought this feature could be open as a start.
> because setting access right and creating user is rather complicated
> for a user without a setup program.

No, no.  We will not have "insecurity by default".  Users will rely on
it, and we'll never be able to close the security hole without
breaking thousands of user installations.

> You didnt understand me. First a pipe creaste with _pipe() is always  
> blocking on windows.
> So if a byte is written to the pipe , the event callback is called and 
> the 256 byte read call
> blocks, because it would only return on eof or if 256 bytes can be read.

Are you really, really sure about that?  I don't believe you, until
you give me a pointer to the documentation.

>> Proper patch welcome.  Don't wait until "later", don't write
>> half-finished code with improper solutions.
>
> recv send and closesocket only used now

Where's the patch?

>>> Well the i removed that because the other code does also compile on  
>>> windows.
>>
>> What other code?
>>
>> You removed the whole host lookup code!  That breaks the
>> bind_to_address setting.
>
> i removed the win32 version

Exactly that was my point.  How do you expect host lookup to work on
Windows, now that you have removed the code?

>> That description belongs to the patch documentation (commit message).
>> It's not obvious what you were trying to achieve, and it's not obvious
>> why the old code was wrong.
>
> hmm i thought it was obvious.

Everything looks obvious to me after I have debugged something for a
few hours.  You don't write documentation for yourself.

>> I suggest you just use autoconf/automake for building, instead of
>> proprietary Visual Studio files.  This way, you don't have to
>> duplicate all the code in Makefile.am.
>
> Sorry but one of MY main goals is to be able to use vs2008 for
> building mpd.

Then it's YOUR task to keep your build files in sync with the official
ones.

> i attach a git diff file as a reference, there are not many changes
> left anymore just 747 lines.

That's everything in one large patch, again.

Max

------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to