[PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

Mark Geisert
I don't have libtirpc in git so I'm submitting a text patch.  Sorry for any
inconvenience.  This is Cygwin-specific and against src/bindresvport.c of
libtirpc 1.0.1.  Unsure if it ought to go upstream; appreciate input on that.
Thanks much,

..mark

--------8<--------
35a36,38
 > /* On Cygwin prefer Cygwin's bindresvport{,_sa}() to portable version here */
 > #if !defined(__CYGWIN__)
 >
247a251,252
 >
 > #endif /* !defined(__CYGWIN__) */
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

Eric Blake (cygwin)-2
On 02/07/2018 01:29 AM, Mark Geisert wrote:

> I don't have libtirpc in git so I'm submitting a text patch.  Sorry for
> any inconvenience.  This is Cygwin-specific and against
> src/bindresvport.c of libtirpc 1.0.1.  Unsure if it ought to go
> upstream; appreciate input on that.
> Thanks much,
>
> ..mark
>
> --------8<--------
> 35a36,38
>  > /* On Cygwin prefer Cygwin's bindresvport{,_sa}() to portable version
> here */

An ed-script diff is practically useless; without context, it is too
easy to misapply the patch if the file has been edited differently in
the meantime.  ALWAYS use 'diff -u' (what git does by default) or 'diff
-c' when generating a patch, so that it has proper context.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

Brian Inglis
On 2018-02-07 08:38, Eric Blake wrote:

> On 02/07/2018 01:29 AM, Mark Geisert wrote:
>> I don't have libtirpc in git so I'm submitting a text patch.  Sorry for any
>> inconvenience.  This is Cygwin-specific and against src/bindresvport.c of
>> libtirpc 1.0.1.  Unsure if it ought to go upstream; appreciate input on that.
>> Thanks much,
>>
>> ..mark
>>
>> --------8<--------
>> 35a36,38
>>  > /* On Cygwin prefer Cygwin's bindresvport{,_sa}() to portable version here */
>
> An ed-script diff is practically useless; without context, it is too easy to
> misapply the patch if the file has been edited differently in the meantime. 
> ALWAYS use 'diff -u' (what git does by default) or 'diff -c' when generating a
> patch, so that it has proper context.

Also mandatory to add -p, --show-c-function for patches, and in general for
directory or recursive patch diffs -N, --new-file so new files are diffed as if
against an empty file; --strip-trailing-cr is useful if some files may have CRs.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

Yaakov Selkowitz
In reply to this post by Mark Geisert
On 2018-02-07 01:29, Mark Geisert wrote:
> I don't have libtirpc in git so I'm submitting a text patch.  Sorry for
> any inconvenience.  This is Cygwin-specific and against
> src/bindresvport.c of libtirpc 1.0.1.  Unsure if it ought to go
> upstream; appreciate input on that.

As Eric mentioned, ed-script diffs are useless.  Nonetheless, I have
been following the discussion, and the correct fix is:

https://github.com/cygwinports/libtirpc/blob/master/1.0.2-cygwin-bindresvport.patch

libtirpc 1.0.2-2 is on its way to the mirrors.

--
Yaakov


signature.asc (235 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

Mark Geisert
In reply to this post by Brian Inglis
Brian Inglis wrote:

> On 2018-02-07 08:38, Eric Blake wrote:
>> On 02/07/2018 01:29 AM, Mark Geisert wrote:
>>> I don't have libtirpc in git so I'm submitting a text patch.  Sorry for any
>>> inconvenience.  This is Cygwin-specific and against src/bindresvport.c of
>>> libtirpc 1.0.1.  Unsure if it ought to go upstream; appreciate input on that.
>>> Thanks much,
>>>
>>> ..mark
>>>
>>> --------8<--------
>>> 35a36,38
>>>  > /* On Cygwin prefer Cygwin's bindresvport{,_sa}() to portable version here */
>>
>> An ed-script diff is practically useless; without context, it is too easy to
>> misapply the patch if the file has been edited differently in the meantime.
>> ALWAYS use 'diff -u' (what git does by default) or 'diff -c' when generating a
>> patch, so that it has proper context.
>
> Also mandatory to add -p, --show-c-function for patches, and in general for
> directory or recursive patch diffs -N, --new-file so new files are diffed as if
> against an empty file; --strip-trailing-cr is useful if some files may have CRs.
>

Understood.  Thanks for the advice.  I knowingly took a risk here on the
assumption nobody else would be working on this specific file.  But with your
advice I don't need to take that kind of risk again.  And the patch will be more
robust too.
Cheers,

..mark
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH libtirpc] Disable libtirpc's own bindresvport{,_sa}() in favor of Cygwin's

Mark Geisert
In reply to this post by Yaakov Selkowitz
Yaakov Selkowitz wrote:

> On 2018-02-07 01:29, Mark Geisert wrote:
>> I don't have libtirpc in git so I'm submitting a text patch.  Sorry for
>> any inconvenience.  This is Cygwin-specific and against
>> src/bindresvport.c of libtirpc 1.0.1.  Unsure if it ought to go
>> upstream; appreciate input on that.
>
> As Eric mentioned, ed-script diffs are useless.  Nonetheless, I have
> been following the discussion, and the correct fix is:
>
> https://github.com/cygwinports/libtirpc/blob/master/1.0.2-cygwin-bindresvport.patch
>
> libtirpc 1.0.2-2 is on its way to the mirrors.
>

Thank you for following the discussion and reworking the patch, Yaakov.
Cheers,

..mark