GSoC AHCI Project

Activity

CR-92 79

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 2h 27m 29 I have uploaded a new diff file https://code.reactos.org/...
    Moderator 8m    
    Reviewer - Complete 1h 4m 9 Do we still need this CR?
    Reviewer - 0% reviewed 10m    
    Reviewer - 0% reviewed      
    Reviewer - 100% reviewed 1h 28m 15 Yes, very good point. DriverEntry needs this and so do al...
    Reviewer - 0% reviewed      
    Reviewer - 100% reviewed 1h 15m 24 Aman Priyadarshi: When you're ready, you can upload a new...
    Reviewer - 0% reviewed      
    Total   6h 32m 79  
    #permalink

    Objectives

    There are no specific objectives for this review.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    HBelusca

    My general questions: *(as I'm not a specialist of ahci stuff): Should this...

    My general questions:

    • (as I'm not a specialist of ahci stuff): Should this driver export functions?
    • Please note that if the WinDDK compiler defaults to creating __stdcall functions (and therefore marking such functions as "NTAPI" is optional), we enforce in our code that all stdcall functions that must be so (because they are exports or part of function table, etc...) need to be marked "NTAPI": for example:
      BOOLEAN
      NTAPI
      IsThisFunctionValid(VOID)
      {
          return TRUE;
      }
      

      This will ensure that the function will always be stdcall when being compiled either with WinDDK or within our build environment (using either MSVC or GCC).


    EDIT: WinDDK headers mostly assume __stdcall is the default calling convention and thus omits to mention it for most of the functions.

    Thomas Faber

    Yes, very good point. DriverEntry needs this and so do all functions that you...

    Yes, very good point. DriverEntry needs this and so do all functions that you pass pointers to (e.g. HwXxx functions)

    HBelusca

    Aman Priyadarshi: When you're ready, you can upload a new patch with all your...

    Aman Priyadarshi: When you're ready, you can upload a new patch with all your modifications, including Alex Ionescu's ones that were commented on your GitHub repository.

    Aman Priyadarshi

    I have uploaded a new diff file https://code.reactos.org/static/olpro3/2stati...

    I have uploaded a new diff file

    Mark Jansen

    Do we still need this CR?

    Do we still need this CR?

    /reactos/drivers/storage/storahci/makefile Added 1
    Open in IDE #permalink
    /reactos/drivers/storage/storahci/sources Added 1
    Open in IDE #permalink
    /reactos/drivers/.../storahci/storahci.c Added 46
    Open in IDE #permalink
    /reactos/drivers/.../storahci/storahci.h Added 18
    Open in IDE #permalink
    /reactos/drivers/.../storahci/storahci.inf Added 8
    Open in IDE #permalink

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against