[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    [Differential] [Commented On] D4376: Replace long-deprecated getpass(3) call
From:       "A. Wilcox" <noreply () phabricator ! kde ! org>
Date:       2017-01-31 19:29:23
Message-ID: 20170131192914.46687.38018.7B3C166D () phabricator ! kde ! org
[Download RAW message or body]

awilcox added a comment.


  > Out of interest: how did you stumble on that code?
  
  Using the musl libc, getpass is not defined unless you enable _GNU_SOURCE:
  
    [ 75%] Building C object kcheckpass/CMakeFiles/kcheckpass.dir/kcheckpass.c.o
    cd /usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build/kcheckpass \
&& /usr/bin/gcc  -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=600 \
-I/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build/kcheckpass \
-I/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass \
-I/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build   \
-DQT_NO_DEBUG -DNDEBUG -O2 -mlong-double-64 -ggdb -mcpu=G3 -fno-omit-frame-pointer  \
-std=iso9899:1990 -fno-common -Wall -Wextra -Wcast-align -Wchar-subscripts \
-Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wmissing-format-attribute \
-Wwrite-strings -Werror=implicit-function-declaration -std=gnu90 -fvisibility=hidden  \
-U_REENTRANT -o CMakeFiles/kcheckpass.dir/kcheckpass.c.o -c \
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c
  /usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c: \
In function ‘conv_legacy':  \
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c:105:10: \
error: implicit declaration of function ‘getpass' \
[-Werror=implicit-function-declaration]  p = getpass(prompt ? prompt : "Password: ");
              ^
    /usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c:105:8: \
warning: assignment makes pointer from integer without a cast [-Wint-conversion]  p = \
getpass(prompt ? prompt : "Password: ");  ^
    cc1: some warnings being treated as errors
    kcheckpass/CMakeFiles/kcheckpass.dir/build.make:62: recipe for target \
'kcheckpass/CMakeFiles/kcheckpass.dir/kcheckpass.c.o' failed  make[2]: *** \
[kcheckpass/CMakeFiles/kcheckpass.dir/kcheckpass.c.o] Error 1  make[2]: Leaving \
directory '/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build'  \
CMakeFiles/Makefile2:2042: recipe for target \
'kcheckpass/CMakeFiles/kcheckpass.dir/all' failed  make[1]: *** \
[kcheckpass/CMakeFiles/kcheckpass.dir/all] Error 2  
  
  
  > If you think there is a risk: better be pedantic in this case. On the other hand \
getdelim should return -1 in error case and then your method returns null. So in my \
book that's good enough error checking.  
  My concern was on the off-chance that getdelim reads a partial password but \
receives EOF before \n so it returns -1.  But, looking at the standard, there is no \
safe way to read/write to the buffer if the return code is -1, so that is actually a \
moot point.

REPOSITORY
  R133 KScreenLocker

REVISION DETAIL
  https://phabricator.kde.org/D4376

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: awilcox
Cc: graesslin, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas


[Attachment #3 (text/html)]

<table><tr><td style="">awilcox added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: \
right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: \
#F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: \
inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D4376" rel="noreferrer">View \
Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid \
#a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; \
background-color: #f8f9fc;"><p>Out of interest: how did you stumble on that \
code?</p></blockquote>

<p>Using the musl libc, getpass is not defined unless you enable _GNU_SOURCE:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" \
data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: \
12px; margin: 0; background: rgba(71, 87, 120, 0.08);">[ 75%] Building C object \
kcheckpass/CMakeFiles/kcheckpass.dir/kcheckpass.c.o cd \
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build/kcheckpass \
&amp;&amp; /usr/bin/gcc  -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=600 \
-I/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build/kcheckpass \
-I/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass \
-I/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build   \
-DQT_NO_DEBUG -DNDEBUG -O2 -mlong-double-64 -ggdb -mcpu=G3 -fno-omit-frame-pointer  \
-std=iso9899:1990 -fno-common -Wall -Wextra -Wcast-align -Wchar-subscripts \
-Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wmissing-format-attribute \
-Wwrite-strings -Werror=implicit-function-declaration -std=gnu90 -fvisibility=hidden  \
-U_REENTRANT -o CMakeFiles/kcheckpass.dir/kcheckpass.c.o -c \
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c
                
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c: \
                In function ‘conv_legacy':
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c:105:10: \
error: implicit declaration of function ‘getpass' \
[-Werror=implicit-function-declaration]  p = getpass(prompt ? prompt : \
&quot;Password: &quot;);  ^
/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5/kcheckpass/kcheckpass.c:105:8: \
warning: assignment makes pointer from integer without a cast [-Wint-conversion]  p = \
getpass(prompt ? prompt : &quot;Password: &quot;);  ^
cc1: some warnings being treated as errors
kcheckpass/CMakeFiles/kcheckpass.dir/build.make:62: recipe for target \
&#039;kcheckpass/CMakeFiles/kcheckpass.dir/kcheckpass.c.o&#039; failed make[2]: *** \
[kcheckpass/CMakeFiles/kcheckpass.dir/kcheckpass.c.o] Error 1 make[2]: Leaving \
directory &#039;/usr/src/kde-plasma/kscreenlocker-5.8.5/work/kscreenlocker-5.8.5_build&#039;
 CMakeFiles/Makefile2:2042: recipe for target \
&#039;kcheckpass/CMakeFiles/kcheckpass.dir/all&#039; failed make[1]: *** \
[kcheckpass/CMakeFiles/kcheckpass.dir/all] Error 2</pre></div>



<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: \
italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>If \
you think there is a risk: better be pedantic in this case. On the other hand \
getdelim should return -1 in error case and then your method returns null. So in my \
book that&#039;s good enough error checking.</p></blockquote>

<p>My concern was on the off-chance that getdelim reads a partial password but \
receives EOF before \n so it returns -1.  But, looking at the standard, there is no \
safe way to read/write to the buffer if the return code is -1, so that is actually a \
moot point.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R133 \
KScreenLocker</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D4376" \
rel="noreferrer">https://phabricator.kde.org/D4376</a></div></div><br \
/><div><strong>EMAIL PREFERENCES</strong><div><a \
href="https://phabricator.kde.org/settings/panel/emailpreferences/" \
rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br \
/><div><strong>To: </strong>awilcox<br /><strong>Cc: </strong>graesslin, \
plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic